Approach B: protocol traits + protocol_impl! macro

[ElFantasma]

Summary

• Adds protocol_impl! declarative macro that implements user-defined protocol traits on ActorRef<A> by mapping methods to messages
• Adds Context::actor_ref() so actors can obtain their own ActorRef from within handlers (needed for self-registration)
• Keeps Response<T> for object-safe async request-response on protocol traits
• Rewrites all cross-boundary examples (chat_room, chat_room_threads, ping_pong, ping_pong_threads) to protocol trait pattern
• Removes actor_api! macro

Shared infrastructure from feat/actor-macro-registry: Handler<M>, #[actor] proc macro, send_messages!/request_messages!, Receiver<M>/Recipient<M>, named registry.

Key pattern: Cross-boundary communication via user-defined trait objects (BroadcasterRef, ParticipantRef). Zero direct dependencies between actor modules — both depend only on a shared protocols.rs.

Test plan

• cargo test -p spawned-concurrency — 34 tests passing
• cargo run -p chat_room
• cargo run -p chat_room_threads
• cargo run -p ping_pong
• cargo run -p ping_pong_threads
• cargo run -p bank
• cargo run -p service_discovery
• git grep "actor_api!" → 0 results in code

[github-actions]

🤖 Kimi Code Review

Review Summary

This is a major API redesign PR that addresses two critical issues (#144 and #145) by replacing the old enum-based Actor trait with a new Handler system. The changes are comprehensive and well-structured.

Key Improvements

1. Type Safety (Improve type safety for request/reply message handling #144): The new Handler<M> trait provides per-message type safety, eliminating the need for large enum matches with impossible variants.

2. Circular Dependencies (Circular dependency problem with bidirectional actor communication #145): The Recipient<M> type (Arc<dyn Receiver<M>>) enables type-erased communication between actors without creating circular module dependencies.

3. Better Ergonomics: The #[actor] macro and actor_api! macro significantly reduce boilerplate while maintaining clarity.

Code Quality

• Correctness: The implementation correctly handles async/await patterns and maintains thread safety
• Error Handling: Proper use of Result types and clear error variants in ActorError
• Memory Safety: No apparent memory leaks or unsafe patterns
• Testing: Comprehensive test suite with 34+ tests covering all backends

Specific Issues Found

1. Potential Race Condition in Registry (concurrency/src/registry.rs:91):

   pub fn registered() -> Vec<String> {
       let store = global_store().read().unwrap_or_else(|p| p.into_inner());
       store.keys().cloned().collect()
   }

   Issue: The RwLock read guard is dropped immediately, but the returned Vec<String> contains cloned keys. Between the read and return, the map could be modified. However, this is acceptable for a snapshot API.

2. Missing Send Bound in Context (concurrency/src/tasks/actor.rs:180):
   The Context<A> struct should probably have A: Send bound for thread safety, though this is likely enforced elsewhere.

3. Potential Panic in Response (concurrency/src/tasks/actor.rs:493):

   ResponseState::Done => panic!("Response polled after completion"),

   Issue: This could panic if Response is polled after completion. Consider using std::task::Poll::Ready(None) or a more graceful error.

Performance Implications

• Envelope Pattern: The new envelope-based dispatch adds a small runtime cost (dynamic dispatch) but enables the type safety benefits
• Memory Usage: Recipient<M> uses Arc<dyn Receiver<M>> which adds one allocation per recipient, but this is reasonable for the flexibility gained

Migration Path

The PR includes complete migration of all examples to the new API, demonstrating backward compatibility isn't maintained (which is acceptable for a 0.x version). The examples show the new API is significantly more ergonomic.

Documentation

• CLAUDE.md: Excellent addition providing project context and usage patterns
• API documentation: The new traits and macros are well-documented with examples

Recommendation

This PR represents a significant improvement to the spawned framework. The new API is more type-safe, more ergonomic, and better aligned with Rust idioms. The implementation is solid with only minor issues that don't affect correctness. Approve with the following suggestions:

1. Consider making the panic in Response more graceful
2. Add more explicit bounds in public APIs where appropriate
3. Consider adding a migration guide for users

---

Automated review by Kimi (Moonshot AI) · custom prompt

[github-actions]

🤖 Codex Code Review

Findings

1. External stop can hang the actor loop (tasks + threads)

   • Context::stop() only cancels the token, but run_actor waits on rx.recv() and only checks cancellation after a message is received. If a caller uses actor_ref.context().stop() while the mailbox is empty, the actor never exits and join() can block indefinitely.
   • A fix is to select on cancellation_token.cancelled() alongside rx.recv() (tasks) and to use a recv_timeout/wakeup mechanism or close the channel on stop (threads).
   • References: concurrency/src/tasks/actor.rs:122-124, concurrency/src/tasks/actor.rs:444-474, concurrency/src/threads/actor.rs:94-96, concurrency/src/threads/actor.rs:367-399.

2. protocol_impl! async requests have no timeout, unlike ActorRef::request

   • The generated request fn returns tasks::Response<T> directly from request_raw without applying the default timeout used by ActorRef::request. That makes protocol traits prone to indefinite waits if a handler never replies or the actor is stuck.
   • Consider adding a request_with_timeout variant in the macro, or document clearly that protocol requests are unbounded and should be wrapped in a timeout by callers.
   • Reference: concurrency/src/message.rs:202-229.

3. Global registry can retain stale actor refs indefinitely

   • registry::register stores values in a process‑global HashMap without any lifecycle integration. If an actor stops, the registry still holds the old ref and whereis will return a dead handle (future sends return ActorStopped).
   • Suggest providing a register_with_guard/RAII helper that unregisters on drop, or integrate unregister on actor stopped() in examples, to avoid stale entries and registry growth.
   • Reference: concurrency/src/registry.rs:5-40.

Notes

• The new Message trait and handler split look clean and more idiomatic, and the updated examples read well.
• I didn’t run tests here.

If you want, I can propose a minimal patch for the cancellation wakeup issue (tasks + threads).

---

Automated review by OpenAI Codex · custom prompt

[greptile-apps]

Greptile Summary

This PR implements "Approach B" for the spawned actor framework, introducing protocol traits with the protocol_impl! macro for type-safe cross-boundary communication.

Key changes:

• Added protocol_impl! declarative macro that implements user-defined protocol traits on ActorRef<A> by mapping trait methods to internal messages
• Added Context::actor_ref() method enabling actors to obtain their own ActorRef for self-registration patterns
• Introduced Response<T> concrete type for object-safe async request-response in protocol traits
• Refactored actor system from enum-based message handlers to per-message Handler<M> trait pattern
• Added #[actor] proc macro to automatically generate Handler implementations from annotated methods
• Introduced named actor registry with register(), whereis(), and unregister() functions
• Simplified error types to ActorStopped and RequestTimeout
• Rewrote all cross-boundary examples (chat_room, ping_pong) to use protocol trait pattern with zero circular dependencies
• Removed deprecated actor_api! macro

Architecture impact:
The protocol trait pattern eliminates circular dependencies between actor modules. Both actors depend only on shared protocol traits (e.g., protocols.rs), not on each other's concrete types. Communication happens through trait objects (Arc<dyn Trait>), enabling clean module boundaries.

All 34 tests pass according to the PR description, and examples have been verified to run correctly.

Confidence Score: 5/5

• This PR is safe to merge - it's a well-designed refactoring with comprehensive test coverage
• The changes represent a major architectural improvement with excellent code quality. All core functionality has parallel implementations for both async (tasks) and sync (threads) modules. The macro implementations are correct and well-documented. Examples demonstrate proper usage patterns. Tests have been updated and verified passing. No security concerns or logical errors detected.
• No files require special attention

Important Files Changed

Filename	Overview
macros/src/lib.rs	Added #[actor] proc macro for generating Handler implementations - clean implementation, properly handles sync/async methods
concurrency/src/message.rs	New file with Message trait and protocol_impl! declarative macro - well-documented, handles all use cases (send/request, async/sync)
concurrency/src/registry.rs	New named actor registry with global HashMap - proper locking, type-safe downcast, good test coverage
concurrency/src/tasks/actor.rs	Major refactor to Handler-based architecture, added Context::actor_ref(), Response<T> for object-safe traits - well-structured changes
concurrency/src/threads/actor.rs	Parallel refactor for threads module with sync handlers - consistent with tasks module design
concurrency/src/error.rs	Simplified error enum to ActorStopped and RequestTimeout - cleaner, more focused API
examples/chat_room/src/protocols.rs	Protocol trait definitions for chat room example - clean separation of concerns, no circular dependencies
examples/chat_room/src/room.rs	ChatRoom actor implementation with protocol_impl! - demonstrates the new pattern effectively
examples/chat_room/src/user.rs	User actor using ctx.actor_ref() for self-registration - demonstrates key new feature properly

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User: Define Protocol Trait] --> B[protocol_impl! Macro]
    B --> C[Generate trait impl on ActorRef]
    
    D[User: Define Actor + Messages] --> E[actor Proc Macro]
    E --> F[Generate Handler impls]
    
    G[Cross-boundary Call] --> H{Protocol Trait Method}
    H -->|send fn| I[ActorRef::send]
    H -->|request fn| J[ActorRef::request_raw]
    J --> K[Response T wrapper]
    
    I --> L[MessageEnvelope]
    K --> L
    L --> M[Actor Message Queue]
    M --> N[Handler::handle]
    N --> O[Actor State Update]
    
    P[ctx.actor_ref] --> Q[Self-registration]
    Q --> R[Pass ActorRef to Other Actors]

Loading

_{Last reviewed commit: 04d86e1}