|
| 1 | +# Bug Fix: Caller Disconnect Causing Callee Exception |
| 2 | + |
| 3 | +## Problem Description |
| 4 | + |
| 5 | +When a WAMP client (caller) calls a remote procedure and exits/disconnects before the call returns, the callee receives an exception and may also exit. This should not happen - the callee should be able to complete the procedure execution normally, and the router should simply discard the result since the caller is no longer available. |
| 6 | + |
| 7 | +## Root Cause |
| 8 | + |
| 9 | +The bug is in `/crossbar/router/dealer.py` in the `detach()` method (lines 185-247). |
| 10 | + |
| 11 | +When a caller disconnects during an in-flight invocation: |
| 12 | + |
| 13 | +1. The router checks if the callee supports call canceling (`call_canceling` feature) |
| 14 | +2. **If the callee does NOT support call canceling:** |
| 15 | + - **OLD BEHAVIOR (BUG)**: The code just does `continue` (line 204) without cleaning up |
| 16 | + - This leaves the invocation tracked in internal data structures: |
| 17 | + - `_invocations` |
| 18 | + - `_callee_to_invocations` |
| 19 | + - `_invocations_by_call` |
| 20 | + - When the callee later returns a result via `processYield()` or `processInvocationError()`, it finds the invocation_request still exists |
| 21 | + - The code tries to send to `invocation_request.caller._transport` which is now `None` |
| 22 | + - This causes an error that propagates to the callee |
| 23 | + |
| 24 | +3. **If the callee DOES support call canceling:** |
| 25 | + - The code properly cleans up the invocation tracking |
| 26 | + - Sends an INTERRUPT message to the callee |
| 27 | + - Everything works correctly |
| 28 | + |
| 29 | +## The Fix |
| 30 | + |
| 31 | +The fix changes the logic to **always clean up the invocation tracking**, regardless of whether the callee supports call canceling or not. The difference is only whether an INTERRUPT message is sent: |
| 32 | + |
| 33 | +```python |
| 34 | +# OLD CODE (BUGGY): |
| 35 | +if not callee_supports_canceling: |
| 36 | + log("INTERRUPT not supported") |
| 37 | + continue # <-- BUG: Skip cleanup! |
| 38 | + |
| 39 | +# Clean up tracking (only executed if callee supports canceling) |
| 40 | +cleanup_invocation_tracking() |
| 41 | +send_interrupt() |
| 42 | +``` |
| 43 | + |
| 44 | +```python |
| 45 | +# NEW CODE (FIXED): |
| 46 | +# Always clean up the invocation tracking |
| 47 | +cleanup_invocation_tracking() |
| 48 | + |
| 49 | +# Only send INTERRUPT if callee supports it |
| 50 | +if callee_supports_canceling: |
| 51 | + log("INTERRUPTing in-flight INVOKE") |
| 52 | + send_interrupt() |
| 53 | +else: |
| 54 | + log("INTERRUPT not supported - invocation cleaned up but not interrupted") |
| 55 | +``` |
| 56 | + |
| 57 | +## Changed Code |
| 58 | + |
| 59 | +**File**: `/crossbar/router/dealer.py` |
| 60 | +**Method**: `Dealer.detach()` |
| 61 | +**Lines**: 192-229 |
| 62 | + |
| 63 | +### Before (buggy): |
| 64 | +```python |
| 65 | +outstanding = self._caller_to_invocations.get(session, []) |
| 66 | +for invoke in outstanding: |
| 67 | + if invoke.canceled: |
| 68 | + continue |
| 69 | + if invoke.callee is invoke.caller: |
| 70 | + continue |
| 71 | + callee = invoke.callee |
| 72 | + |
| 73 | + # BUG: If callee doesn't support canceling, skip cleanup entirely |
| 74 | + if not callee_supports_call_canceling(callee): |
| 75 | + self.log.debug("INTERRUPT not supported...") |
| 76 | + continue # <-- SKIPS CLEANUP! |
| 77 | + |
| 78 | + # Cleanup only happens if we get past the continue |
| 79 | + cleanup_invocation() |
| 80 | + send_interrupt() |
| 81 | +``` |
| 82 | + |
| 83 | +### After (fixed): |
| 84 | +```python |
| 85 | +outstanding = self._caller_to_invocations.get(session, []) |
| 86 | +for invoke in outstanding: |
| 87 | + if invoke.canceled: |
| 88 | + continue |
| 89 | + if invoke.callee is invoke.caller: |
| 90 | + continue |
| 91 | + callee = invoke.callee |
| 92 | + |
| 93 | + # ALWAYS clean up invocation tracking |
| 94 | + if invoke.timeout_call: |
| 95 | + invoke.timeout_call.cancel() |
| 96 | + invoke.timeout_call = None |
| 97 | + |
| 98 | + invokes = self._callee_to_invocations[callee] |
| 99 | + invokes.remove(invoke) |
| 100 | + if not invokes: |
| 101 | + del self._callee_to_invocations[callee] |
| 102 | + |
| 103 | + del self._invocations[invoke.id] |
| 104 | + del self._invocations_by_call[(invoke.caller_session_id, invoke.call.request)] |
| 105 | + |
| 106 | + # ONLY send INTERRUPT if callee supports it |
| 107 | + if callee_supports_call_canceling(callee): |
| 108 | + self.log.debug("INTERRUPTing in-flight INVOKE...") |
| 109 | + self._router.send(invoke.callee, message.Interrupt(...)) |
| 110 | + else: |
| 111 | + self.log.debug("INTERRUPT not supported - invocation cleaned up but not interrupted") |
| 112 | +``` |
| 113 | + |
| 114 | +## Impact |
| 115 | + |
| 116 | +### What Changes: |
| 117 | +- **Callee behavior when caller disconnects without call_canceling support:** |
| 118 | + - **BEFORE**: Invocation stays tracked, callee gets exception when returning result |
| 119 | + - **AFTER**: Invocation is cleaned up silently, callee can complete normally, result is discarded |
| 120 | + |
| 121 | +### What Stays the Same: |
| 122 | +- Callees that support `call_canceling` still receive INTERRUPT messages as before |
| 123 | +- All other WAMP routing behavior unchanged |
| 124 | +- Backward compatible with existing deployments |
| 125 | + |
| 126 | +## Testing |
| 127 | + |
| 128 | +Existing test coverage: |
| 129 | +- `test_caller_detach_interrupt_cancel_not_supported` - Tests that no INTERRUPT is sent when callee doesn't support canceling |
| 130 | +- `test_caller_detach_interrupt_cancel_supported` - Tests that INTERRUPT is sent when callee supports canceling |
| 131 | + |
| 132 | +The fix preserves the behavior verified by these tests while fixing the cleanup issue. |
| 133 | + |
| 134 | +## Recommendation |
| 135 | + |
| 136 | +This is a critical bug fix that prevents callee crashes when callers disconnect. It should be: |
| 137 | +1. Applied to the current development branch |
| 138 | +2. Backported to any maintained release branches |
| 139 | +3. Included in the next patch release |
| 140 | + |
| 141 | +## Technical Details |
| 142 | + |
| 143 | +### Data Structures Involved: |
| 144 | +- `_caller_to_invocations[session]` - Maps caller session to list of in-flight invocations |
| 145 | +- `_callee_to_invocations[session]` - Maps callee session to list of in-flight invocations |
| 146 | +- `_invocations[invocation_id]` - Maps invocation ID to InvocationRequest |
| 147 | +- `_invocations_by_call[(caller_session_id, call_request_id)]` - Maps call request to invocation |
| 148 | + |
| 149 | +### WAMP Protocol Compliance: |
| 150 | +According to the WAMP specification, when a caller disconnects during an RPC: |
| 151 | +- If the callee supports call canceling, it should receive an INTERRUPT |
| 152 | +- If the callee doesn't support call canceling, it continues execution normally |
| 153 | +- In both cases, any result from the callee should be discarded by the router |
| 154 | + |
| 155 | +The old code violated this by leaving stale invocation tracking that would cause errors when the callee tried to return a result. |
0 commit comments