Skip to content

Conversation

@parth-soni07
Copy link
Contributor

What was wrong?

The codebase previously relied on a custom async_service implementation, which was no longer actively maintained and introduced unnecessary complexity. It made lifecycle management fragile and lacked support for modern async patterns and structured concurrency.

Issue #524

How was it fixed?

The legacy async_service implementation was fully replaced with a new design built using anyio, a well-supported, high-level async framework. This transition modernizes the service infrastructure and significantly improves reliability, maintainability, and task supervision.

@seetadev
Copy link
Contributor

@parth-soni07 : Thank you so much for sharing this PR. Appreciate your great efforts and contribution. We will jointly do a detailed review. Will share feedback soon.

Wish if you could resolve the merge conflicts. Once resolved, please ping me.

I will re-run the CI/CD pipeline and share test results soon.

Also, CCing @acul71, @sumanjeet0012 and @pacrob.

@sumanjeet0012
Copy link
Contributor

@parth-soni07 I have reviewed the PR and have a few observations:
1. The PR title indicates that we are creating an async service using AnyIO, but the implementation uses Trio instead.
2. The test directory is still named async_service instead of anyio_service.
3. The newsfragment file appears to be missing.

@parth-soni07
Copy link
Contributor Author

@parth-soni07 I have reviewed the PR and have a few observations: 1. The PR title indicates that we are creating an async service using AnyIO, but the implementation uses Trio instead. 2. The test directory is still named async_service instead of anyio_service. 3. The newsfragment file appears to be missing.

Hey @sumanjeet0012 , thank you for your insights, yes indeed this is supposed to be a anyio migration and not trio, I have made some changes in the implementation to align with the name of the PR and use anyio. I am facing some issues in the version of anyio that is used in the p2pclient so currently working on that, will definitely ping once I am done with that.

@acul71
Copy link
Contributor

acul71 commented Nov 14, 2025

Hello @parth-soni07 thanks for this PR.
There are some lint-typecheck-doc-issues to fix.
Ping if blocked.

AI Pull Request Review: PR #973

PR Title: Replace async service with aniyo service
Author: parth-soni07
Related Issue: #524
Related Discussion: #1008
Status: OPEN
Date: November 2025


1. Summary of Changes

This PR implements a major refactoring that replaces the legacy async_service implementation with a new anyio_service framework built on top of AnyIO. This addresses issue #524, which identified that async_service was based on an unmaintained library and needed modernization.

Understanding the Service Framework Ecosystem

To understand the significance of this migration, it's important to understand the role of service frameworks in py-libp2p and the broader async Python ecosystem:

async_service (Legacy - Being Replaced)

The original async_service was a service lifecycle management framework that py-libp2p copied from the ethereum/async-service library to eliminate an external dependency. This framework provided:

  • Service lifecycle management: Start, stop, cancel, and wait operations for long-running async services
  • Task supervision: Spawning and managing child tasks with proper cleanup
  • Error handling: Collecting and propagating errors from concurrent tasks
  • Statistics tracking: Monitoring task counts and service state

However, the upstream library became unmaintained, leaving py-libp2p with a frozen codebase that lacked modern async patterns, bug fixes, and improvements. The code was tightly coupled to Trio's async primitives (nurseries, cancellation scopes) and didn't leverage newer structured concurrency patterns.

Trio (The Async Runtime)

Trio is a modern, production-grade async framework for Python that emphasizes:

  • Structured concurrency: Tasks are organized in a tree structure with clear parent-child relationships
  • Nurseries: Context managers that ensure all spawned tasks complete before exiting
  • Cancellation: Built-in cancellation propagation through the task tree
  • Deterministic cleanup: Guaranteed resource cleanup even when errors occur

py-libp2p has been built on Trio from the beginning because its structured concurrency model aligns perfectly with libp2p's needs:

  • Network services (Swarm, PubSub, KadDHT) need to manage many concurrent connections
  • Connection lifecycle requires proper cleanup when peers disconnect
  • Protocol handlers spawn child tasks that must be supervised and cleaned up
  • Error isolation prevents one failing connection from crashing the entire node

All core py-libp2p services (Swarm, PubSub, KadDHT, Circuit Relay) inherit from the Service base class and use the service framework to manage their lifecycle.

AnyIO (The Compatibility Layer)

AnyIO is a compatibility library that provides a unified async API that works with multiple backends:

  • Trio backend: Uses Trio's primitives (nurseries, cancellation scopes)
  • asyncio backend: Uses asyncio's primitives (TaskGroups, cancellation)

AnyIO's key benefits:

  • Cross-platform compatibility: Code can run on either Trio or asyncio without changes
  • Modern structured concurrency: Provides create_task_group() similar to Trio's nurseries
  • Active maintenance: Well-maintained with regular updates and bug fixes
  • ExceptionGroup support: Modern error handling for concurrent operations (Python 3.11+)

anyio_service (The New Implementation)

The new anyio_service framework is a complete rewrite that:

  1. Uses AnyIO as the foundation: Provides cross-platform async compatibility while maintaining Trio as the primary backend for py-libp2p
  2. Maintains backward compatibility: The background_trio_service() function still exists and uses TrioManager internally, ensuring existing code continues to work
  3. Offers future flexibility: The new background_anyio_service() function uses AnyIOManager, which could theoretically work with asyncio if needed in the future
  4. Improves architecture:
    • Modular design with clear separation of concerns
    • Better error handling with ExceptionGroup support
    • Enhanced statistics and monitoring
    • Improved task hierarchy management

The Relationship in py-libp2p

In py-libp2p's architecture:

┌─────────────────────────────────────────────────────────┐
│  py-libp2p Services (Swarm, PubSub, KadDHT, etc.)      │
│  └─ Inherit from Service base class                    │
│     └─ Use background_trio_service() context manager   │
│        └─ Uses TrioManager (Trio-based)                │
│           └─ Manages lifecycle via Trio nurseries      │
│              └─ Runs on Trio async runtime             │
└─────────────────────────────────────────────────────────┘

Current State (After this PR):

  • All services still use background_trio_service()TrioManager → Trio primitives
  • The new anyio_service package provides both TrioManager and AnyIOManager
  • TrioManager is a wrapper that uses pure Trio primitives for maximum compatibility
  • AnyIOManager uses AnyIO primitives but can still run on the Trio backend
  • The migration is transparent to existing code - same API, better implementation

Why This Matters:

  • Maintainability: Active upstream support and modern patterns
  • Reliability: Better error handling and resource cleanup
  • Future-proofing: Potential to support asyncio if needed without code changes
  • Code quality: Cleaner, more modular architecture that's easier to test and extend

What Changed:

  • Replaced libp2p/tools/async_service/ with libp2p/tools/anyio_service/

    • New modular architecture with separate modules: api.py, manager.py, context.py, exceptions.py, stats.py, tasks.py, utils.py
    • Added trio_manager.py and trio_tasks.py for Trio-specific implementations
    • Maintains backward compatibility through background_trio_service() function
  • Updated all imports across the codebase (30+ files):

    • Core modules: libp2p/network/swarm.py, libp2p/host/basic_host.py, libp2p/pubsub/, libp2p/kad_dht/, libp2p/relay/circuit_v2/
    • Examples: examples/kademlia/, examples/pubsub/, examples/random_walk/
    • Tests: All test files updated to use new imports
    • Documentation: Updated ReST documentation files
  • Added dependency management:

  • Architecture improvements:

    • Uses AnyIO for cross-platform async compatibility (asyncio + trio backends)
    • Better structured concurrency with proper task supervision
    • Improved lifecycle management with explicit state tracking
    • Enhanced error handling and statistics collection

Breaking Changes:

  • API Change: Import paths changed from libp2p.tools.async_service to libp2p.tools.anyio_service
  • Internal: Service implementation details changed, but public API (Service, background_trio_service) remains compatible

2. Strengths

  1. Comprehensive Migration: All imports have been systematically updated across the entire codebase, including examples, tests, and documentation.

  2. Modular Design: The new anyio_service package is well-organized with clear separation of concerns:

    • api.py: Public interfaces and decorators
    • manager.py: AnyIO-based manager implementation
    • trio_manager.py: Trio-specific manager for backward compatibility
    • context.py: Context managers for running services
    • exceptions.py, stats.py, tasks.py, utils.py: Supporting modules
  3. Backward Compatibility: The background_trio_service() function is preserved, ensuring existing code continues to work without changes.

  4. Modern Async Framework: Migration to AnyIO provides:

    • Cross-platform compatibility (asyncio + trio)
    • Better structured concurrency primitives
    • Active maintenance and community support
    • Improved error handling with ExceptionGroup support
  5. Documentation Updates: Documentation has been updated to reflect the new module structure, including new ReST files for libp2p.tools.anyio_service.

  6. Dependency Management: Addition of constraints.txt addresses the anyio version conflict mentioned in discussion Resolving `anyio` Dependency Conflict in `p2pclient` During Migration #1008.

  7. Test Coverage: New test files exist for anyio_service:

    • test_anyio_based_service.py
    • test_anyio_external_api.py
    • test_anyio_manager_stats.py

3. Issues Found

Critical

  1. File: libp2p/tools/anyio_service/api.py
    Line(s): 12
    Issue: RUNTIME IMPORT ERROR - CRITICAL BLOCKER
    Details: Cannot import ObjectReceiveStream and ObjectSendStream from anyio.abc. This prevents the code from running at all - all tests, documentation builds, and runtime execution fail.
    Suggestion: Verify the correct import path for these classes in the AnyIO version being used. These classes may not exist, have different names, or be in a different module. See section 7 for detailed analysis.

Major

  1. File: newsfragments/
    Line(s): N/A
    Issue: MISSING NEWSFRAGMENT - BLOCKER
    Suggestion: A newsfragment file is MANDATORY for PR approval. This PR addresses issue Assess the need for replacing for async_service #524, so a file named 524.feature.rst (or 524.internal.rst if this is considered an internal change) must be created in the newsfragments/ directory. The file should contain a user-facing description of the change and end with a newline character.

  2. File: libp2p/tools/async_service/
    Line(s): N/A
    Issue: The old async_service directory still exists (contains only __pycache__/)
    Suggestion: The directory should be completely removed to avoid confusion. If there are any remaining files, they should be deleted. Only the __pycache__/ directory remains, which should be cleaned up.

  3. File: tests/core/tools/anyio_service/test_anyio_based_service.py
    Line(s): 23-27
    Issue: All tests in this file are skipped with pytestmark = pytest.mark.skip
    Suggestion: The comment indicates "AnyIO manager needs additional work - use trio tests instead". This suggests the AnyIO implementation may not be fully tested. Either:

    • Complete the AnyIO implementation and enable these tests, or
    • Document why these tests are skipped and when they will be enabled, or
    • Remove the tests if they're not ready
  4. File: docs/conf.py
    Line(s): 18
    Issue: Missing newline at end of file (diff shows \ No newline at end of file)
    Suggestion: Add a trailing newline to comply with project standards.

Minor

  1. File: PR Title
    Line(s): N/A
    Issue: Typo in PR title: "aniyo" should be "anyio"
    Suggestion: Fix the typo in the PR title: "Replace async service with anyio service"

  2. File: constraints.txt
    Line(s): 1
    Issue: The constraint file only contains anyio>=4.11.0 but discussion Resolving `anyio` Dependency Conflict in `p2pclient` During Migration #1008 mentions conflicts with p2pclient
    Suggestion: Consider if additional constraints are needed or if this will be handled separately. The discussion mentions p2pclient uses an internal anyio version that conflicts. This may need coordination with the p2pclient fork mentioned in discussion Resolving `anyio` Dependency Conflict in `p2pclient` During Migration #1008.

  3. File: libp2p/tools/anyio_service/context.py
    Line(s): 64
    Issue: Type ignore comment: # type: ignore[arg-type]
    Suggestion: Investigate if the type issue can be resolved properly rather than ignored, or document why the ignore is necessary.


4. Security Review

Security Impact: None / Low

No security vulnerabilities identified. The changes are primarily architectural refactoring:

  • No external input validation changes: The service framework doesn't handle external input directly
  • No cryptographic changes: No changes to key handling or cryptographic operations
  • No subprocess or OS operations: No new subprocess calls or file operations introduced
  • Error handling: Improved error handling with ExceptionGroup support may actually improve security by ensuring errors are properly propagated

Potential Considerations:

  • The migration to AnyIO should be tested thoroughly to ensure no regressions in connection handling or resource cleanup
  • Task supervision improvements should prevent resource leaks, which is a security-positive change

5. Documentation and Examples

Strengths:

  • ✅ Documentation files updated (docs/libp2p.tools.anyio_service.rst created)
  • ✅ Old documentation removed (docs/libp2p.tools.async_service.rst deleted)
  • ✅ Example files updated with new imports
  • ✅ Module docstrings present in new anyio_service package
  • ✅ Good module-level docstring in libp2p/tools/anyio_service/__init__.py explaining the package structure
  • ✅ Both background_trio_service() and background_anyio_service() functions in context.py have clear docstrings explaining their purpose and differences

Issues:

  1. Missing: Migration guide or changelog entry explaining the change for users
    Suggestion: While the newsfragment will cover this, consider adding a brief migration note if there are any breaking changes users need to be aware of (beyond import path changes).

6. Newsfragment Requirement

⚠️ CRITICAL: Newsfragment is MISSING - This is a BLOCKER for PR approval.

Required Action:

File: newsfragments/524.feature.rst (or 524.internal.rst)

Content should be:

Replaced legacy async_service implementation with modern anyio_service framework built on AnyIO for better structured concurrency, cross-platform compatibility, and improved maintainability.

Requirements:

  • ✅ Filename format: 524.<TYPE>.rst where TYPE is feature (new functionality) or internal (internal refactoring)
  • ❌ File does not exist
  • ✅ Must end with newline character
  • ✅ Must be user-facing description (not implementation details)

Type Selection:

  • Use .feature.rst if this is considered new functionality (modern async framework)
  • Use .internal.rst if this is considered an internal refactoring (implementation change only)

Impact: PR cannot be approved without this newsfragment file. This is a mandatory requirement, not optional.


7. Tests and Validation

Test Coverage:

New Tests Added:

  • tests/core/tools/anyio_service/test_anyio_based_service.py (but skipped)
  • tests/core/tools/anyio_service/test_anyio_external_api.py
  • tests/core/tools/anyio_service/test_anyio_manager_stats.py

Test Updates:

  • ✅ All existing tests updated to use new import paths
  • ✅ Test utilities updated (tests/utils/factories.py)

Issues:

  1. Skipped Tests: The main AnyIO service tests are skipped with the reason "AnyIO manager needs additional work - use trio tests instead". This raises questions about the completeness of the AnyIO implementation.

  2. Test Directory Naming: Reviewer @sumanjeet0012 noted that the test directory is still named async_service instead of anyio_service. However, the diff shows tests are in tests/core/tools/anyio_service/, so this may have been addressed.

Build and Linting:

Status:FAILED - Both make pr and make test were executed and found critical runtime errors.

Results Summary:

✅ Passed:

  • Ruff linting and formatting checks
  • YAML/TOML validation
  • File formatting (trailing whitespace, end of files)
  • pyupgrade checks
  • Markdown formatting
  • Pre-commit hooks (fixed missing newline in docs/conf.py)

❌ Failed:

  • mypy type checking: 29 errors in 4 files
  • pyrefly type checking: 56 errors (similar issues)

Type Checking Errors Found:

The type checkers (mypy and pyrefly) are reporting multiple issues with the AnyIO integration:

  1. Missing AnyIO type stubs/attributes:

    • anyio.abc.ObjectReceiveStream and ObjectSendStream not found
    • anyio.create_memory_object_stream not found
    • anyio.sleep_forever not found (used in tests)
  2. TaskGroup API issues:

    • TaskGroup.start_soon() not recognized (but exists at runtime)
    • Multiple instances across api.py, manager.py, context.py, and tests
  3. Abstract class instantiation:

    • Cannot instantiate abstract Event class
    • Cannot instantiate abstract CancelScope class
    • Missing __enter__/__exit__ on CancelScope (should use __aenter__/__aexit__)
  4. Missing await statements:

    • Several coroutines not being awaited (e.g., anyio.current_task() returns coroutine)
    • cancel() calls on potentially None objects
  5. Return type mismatches:

    • Functions returning Literal[False] | None but declared to return bool

Root Cause Analysis:
These errors suggest either:

  • Incomplete type stubs for AnyIO in the type checker's environment
  • Version mismatch between runtime AnyIO and type stubs
  • Actual bugs in the code (missing awaits, incorrect API usage)
  • Type checker limitations with AnyIO's dynamic API

Recommendation:

  • Verify AnyIO version matches type stubs
  • Add proper type ignores where runtime behavior is correct but type checker can't verify
  • Fix actual bugs (missing awaits, None checks)
  • Consider adding # type: ignore comments with specific error codes for known type checker limitations

Action Required: These type checking errors must be resolved before merge, as they may indicate actual runtime issues or will cause CI to fail.

Runtime Import Errors (CRITICAL):

Status:FAILED - make test was executed and found critical import errors that prevent the code from running.

Error:

ImportError: cannot import name 'ObjectReceiveStream' from 'anyio.abc'
ImportError: cannot import name 'ObjectSendStream' from 'anyio.abc'

Impact:

  • All tests fail during import phase - cannot even collect tests
  • The code cannot run at all - this is a blocker
  • Affects the entire codebase since libp2p/__init__.py imports from anyio_service

Root Cause:
The imports in libp2p/tools/anyio_service/api.py:12 are incorrect:

from anyio.abc import ObjectReceiveStream, ObjectSendStream

These classes either:

  1. Don't exist in the installed AnyIO version
  2. Are in a different module location
  3. Have different names in the AnyIO API

Files Affected:

  • libp2p/tools/anyio_service/api.py:12 - The import statement causing the failure
  • All test files fail to import because the module cannot be loaded

Action Required:

  • CRITICAL BLOCKER - This must be fixed before any tests can run
  • Verify the correct import path for ObjectReceiveStream and ObjectSendStream in the AnyIO version being used
  • Check AnyIO documentation/API for the correct way to import these types
  • This confirms that the type checking errors were indicating real runtime problems

8. Recommendations for Improvement

  1. CRITICAL - Add Newsfragment:

    • Create newsfragments/524.feature.rst (or 524.internal.rst) with user-facing description
    • Ensure it ends with a newline character
  2. Clean Up Old Directory:

    • Remove libp2p/tools/async_service/ directory completely (currently only contains __pycache__/)
  3. Fix PR Title Typo:

    • Change "aniyo" to "anyio" in the PR title
  4. Address Skipped Tests:

    • Either complete the AnyIO implementation and enable tests, or document the roadmap for enabling them
    • Consider if the AnyIO manager is production-ready if its tests are skipped
  5. Fix Missing Newline:

    • FIXED - Pre-commit hook automatically fixed the missing newline in docs/conf.py
  6. CRITICAL - Fix Import Errors (BLOCKER):

    • Runtime import failure: ObjectReceiveStream and ObjectSendStream cannot be imported from anyio.abc
    • This prevents the code from running at all - all tests fail during import
    • Verify the correct import path for these classes in the AnyIO version being used
    • Check if these classes exist, have different names, or are in a different module
    • This is the highest priority fix - nothing else can be tested until this is resolved
  7. CRITICAL - Fix Type Checking Errors:

    • 29 mypy errors and 56 pyrefly errors must be resolved before merge
    • The import errors confirm these were indicating real runtime problems
    • Investigate and fix missing await statements (e.g., anyio.current_task())
    • Add proper None checks before calling methods on potentially None objects
    • Fix return type mismatches (functions returning None but declared to return bool)
    • Add appropriate # type: ignore comments for known type checker limitations with AnyIO
    • Verify AnyIO version compatibility with type stubs
  8. Verify Build:

    • make pr was run - found type checking failures (see above)
    • make test was run - FAILED with critical import errors (see above)
    • make linux-docs was run - FAILED with 138 warnings (treated as errors) due to the same import errors
    • ⚠️ Run full test suite after fixing import errors to ensure no regressions
  9. Coordinate with Discussion Resolving `anyio` Dependency Conflict in `p2pclient` During Migration #1008:

  10. Type Ignore Comments:

  • Investigate the # type: ignore[arg-type] in context.py:64 and resolve if possible, or document why it's necessary

9. Questions for the Author

  1. Test Completeness: Why are the AnyIO-based service tests skipped? Is the AnyIO manager implementation complete and production-ready, or is it still a work in progress?

  2. Backward Compatibility: The PR maintains background_trio_service() for compatibility. Are there any plans to deprecate this in favor of background_anyio_service(), or will both be maintained long-term?

  3. p2pclient Dependency: Discussion Resolving `anyio` Dependency Conflict in `p2pclient` During Migration #1008 mentions a dependency conflict with p2pclient. Has this been resolved with the constraints.txt approach, or does it need additional work?

  4. Migration Strategy: Are there any migration steps users need to take beyond updating import paths? Should there be a migration guide?

  5. Performance Impact: Has the migration to AnyIO been benchmarked? Are there any performance implications (positive or negative) compared to the old async_service?

  6. Error Handling: The new implementation uses ExceptionGroup. How does this affect error handling in existing code? Are there any breaking changes in exception types?

  7. Resource Management: The new implementation has improved task supervision. Are there any changes in how resources (connections, streams) are managed or cleaned up?


10. Overall Assessment

Quality Rating: Needs Work

The PR represents a well-structured migration with comprehensive code updates, good modular design, and maintained backward compatibility. However:

  • Critical runtime import errors prevent the code from running at all
  • Missing newsfragment is a blocker for approval
  • Type checking errors indicate real runtime problems

The architectural design is sound, but the implementation has critical issues that must be resolved before merge.

Security Impact: None / Low

No security concerns identified. The refactoring improves error handling and resource management.

Merge Readiness: Needs fixes

Blockers:

  • ❌ Missing newsfragment file (CRITICAL - mandatory requirement)
  • Runtime import errors - Code cannot run at all (CRITICAL BLOCKER)
  • ❌ Type checking errors (29 mypy + 56 pyrefly errors) - must be resolved before merge

Before Merge:

  • ✅ Add newsfragment for issue Assess the need for replacing for async_service #524
  • ✅ Remove old async_service/ directory
  • ✅ Fix PR title typo ("aniyo" → "anyio")
  • ✅ Fix missing newline in docs/conf.py (FIXED by pre-commit hook)
  • CRITICAL: Fix import errors - ObjectReceiveStream and ObjectSendStream cannot be imported from anyio.abc (see section 7)
  • ❌ Fix type checking errors (see section 7 for details)
  • ❌ Fix runtime import errors preventing tests and documentation from running
  • ❌ Documentation build fails with 138 warnings due to import errors
  • ⚠️ Address skipped tests or document roadmap

After Merge Considerations:

Confidence: Low

The code changes are well-structured and comprehensive architecturally, but:

  • Critical runtime import errors prevent the code from running at all
  • Type checking errors were indicating real runtime problems (confirmed by import failures)
  • Missing newsfragment is a straightforward fix
  • Skipped tests raise questions about implementation completeness

The import errors are a critical blocker - the code cannot be tested or used until these are fixed. This suggests the AnyIO integration may not have been fully tested or the API usage is incorrect for the AnyIO version being used.


Additional Context from Discussion #1008

The related discussion #1008 highlights a dependency conflict with p2pclient which uses an internal anyio version. The discussion mentions:

  • p2pclient's latest version was released February 9, 2022 (unmaintained)
  • Plan to fork p2pclient and update anyio dependency
  • The constraints.txt file in this PR may help, but coordination with the p2pclient fork is needed

This context is important for understanding the dependency management strategy and ensuring the anyio version constraint works across all dependencies.


Review completed: 2025-11-13
Reviewer: AI Assistant (following py-libp2p PR review prompt)
Verification:

  • ✅ Type checking errors verified: 29 mypy errors confirmed
  • ✅ Runtime import errors verified: Code cannot run due to missing AnyIO imports
  • ✅ Documentation build verified: Fails with 138 warnings due to import errors
  • ✅ Test execution verified: All tests fail during import phase

Next Steps: Author should address blockers, particularly the critical import errors (highest priority) and missing newsfragment, before requesting re-review.

yashksaini-coder and others added 7 commits November 15, 2025 13:07
- Introduced hashlib for SHA-256 hashing in routing_table.py, utils.py, and transport.py.
- Updated peer ID generation in id.py to use SHA-256 for larger keys and identity multihash for smaller keys.
- Adjusted tests in test_peerid.py to reflect the new hashing approach.
- Added missing import statements for hashlib where necessary.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants