Skip to content

Commit b907681

Browse files
tclemCopilot
andcommitted
docs(skill): clarify that handler thread-safety is compiler-enforced
The "anti-pattern" framing in examples.md was misleading -- it implied the RefCell example might compile but produce bugs at runtime. In reality, SessionHandler's `Send + Sync + 'static` trait bound rejects any handler whose state contains a non-Sync type (RefCell, Cell, Rc), so the example fails at the impl site, not at use. Reframe as "Won't compile", show the actual trait-bound error inline, and add a one-paragraph note explaining the compiler-level enforcement so a reader doesn't have to mentally trace `spawn -> Send + Sync -> RefCell !Sync`. Addresses stephentoub's review feedback that the prior framing read as "we've buried some unsafe usage." Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c8fe373 commit b907681

1 file changed

Lines changed: 17 additions & 3 deletions

File tree

.github/skills/rust-coding-skill/examples.md

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,19 +85,33 @@ tokio::spawn(async move {
8585
on spawned tasks (see `rust/src/session.rs:973` and `:1022`). Implementations
8686
must be safe for concurrent invocation.
8787

88-
### Anti-pattern — non-`Send` mutable state in the handler
88+
The `SessionHandler` trait declares `Send + Sync + 'static`, so the compiler
89+
enforces this — handlers with non-`Sync` state (e.g. `RefCell`, `Cell`,
90+
`Rc`) won't compile. The examples below make the rejection mechanism explicit.
91+
92+
### Won't compile — non-`Sync` state
8993

9094
```rust
9195
struct MyHandler {
92-
last_request: std::cell::RefCell<Option<String>>, // not thread-safe
96+
last_request: std::cell::RefCell<Option<String>>, // RefCell: !Sync
97+
}
98+
99+
#[async_trait]
100+
impl SessionHandler for MyHandler {
101+
// ^^^^^^^^^^^^^^ the trait `Sync` is not implemented for `RefCell<...>`
102+
async fn on_event(&self, event: HandlerEvent) -> HandlerResponse { /* ... */ }
93103
}
94104
```
95105

106+
The error surfaces at the `impl` site, not at use site, because the trait's
107+
`Send + Sync` bound makes `RefCell` ineligible for any field of any type that
108+
implements `SessionHandler`.
109+
96110
### Preferred — `parking_lot::Mutex` or atomics
97111

98112
```rust
99113
struct MyHandler {
100-
last_request: parking_lot::Mutex<Option<String>>,
114+
last_request: parking_lot::Mutex<Option<String>>, // Mutex<T>: Sync if T: Send
101115
}
102116
```
103117

0 commit comments

Comments
 (0)