Make Mbient.close() safe against live streaming callbacks

[lwhite1]

Background

At normal session end, mbients are still actively streaming data when TerminateServerRequest arrives. Confirmed by tracing the session flow:

• CTR sends PerformTaskRequest per task, followed by TasksFinished
• Per-task StopRecording calls stop_recording() → device_manager.stop_recording_devices() → _get_camera_devices() — only cameras are stopped
• Mbients run continuously from create_streams() through to close_streams() on terminate
• When the GUI shows "safe to terminate" and the user clicks terminate, mbient data callbacks are still firing on native threads

This means our current Mbient.close() flow (after PR #648):

1. Unsubscribe from the fusion processor — while data callbacks are still firing on native threads
2. stop() → stop_inertial_sampling() → disable_inertial_sampling()
3. Disconnect

Step 1 races against in-flight native callbacks. PR #648 fixed the SIGILL from unsubscribing after disable, but did not address the unsubscribe-vs-callback race.

Three improvements

1. Stop data flow before unsubscribe (local, highest impact)

Split stop() into two phases with unsubscribe between them:

def close(self) -> None:
    # Phase 1: stop data generation (processor still valid, callbacks drain)
    try:
        if self.streaming:
            self.device_wrapper.stop_inertial_sampling()
            time.sleep(0.05)  # let in-flight callbacks drain
    except Exception: ...

    # Phase 2: unsubscribe (no active callbacks, processor still valid)
    for signal in self.subscribed_signals:
        try:
            libmetawear.mbl_mw_datasignal_unsubscribe(signal)
        except Exception: ...
    self.subscribed_signals.clear()

    # Phase 3: power down sensors (nothing references processor)
    try:
        if self.streaming:
            self.device_wrapper.disable_inertial_sampling()
            self.streaming = False
            self.state = DeviceState.STOPPED
    except Exception: ...

    self.disconnect()

This matches the safest MetaWear lifecycle: stop → drain → unsubscribe → disable.

2. Call mbl_mw_dataprocessor_remove after unsubscribe (local, needs verification)

The fusion processor created at mbient.py:304 via mbl_mw_dataprocessor_fuser_create is never freed. mbl_mw_datasignal_unsubscribe only detaches the callback — the processor object continues to exist on the native side. The SDK exposes mbl_mw_dataprocessor_remove(processor) for this (present in MetaWear cbindings).

One-line addition in the unsubscribe loop:

for signal in self.subscribed_signals:
    try:
        libmetawear.mbl_mw_datasignal_unsubscribe(signal)
        libmetawear.mbl_mw_dataprocessor_remove(signal)
    except Exception: ...

Needs verification against MetaWear SDK examples before shipping — calling this incorrectly could introduce a new crash mode.

3. Stop mbients before close_streams (architectural, low risk)

Currently the GUI shows "safe to terminate" purely because tasks are done — but mbient streams are still live. We should explicitly stop mbients before entering the crash-prone close_streams() path.

Simplest approach: the ACQ terminate handler stops mbients synchronously before calling close_streams():

if device_manager is not None:
    for name, stream in device_manager.get_mbient_streams().items():
        try:
            stream.stop()  # stops + disables, sets streaming=False
        except Exception as e:
            logger.error(f'Error stopping mbient {name}: {e}')
    device_manager.close_streams()

By the time close_streams() runs, mbients are already stopped and no callbacks are in flight.

Alternative: new StopWearables message from CTR to each ACQ before TerminateServerRequest. More elaborate but cleaner separation.

Recommendation

Ship #1 and #3 together. They combine to give belt-and-suspenders protection:

• Read task parameters from database #3 ensures mbients are stopped before close_streams() runs
• Folder structure #1 ensures close() itself handles a live-streaming mbient safely (covers the edge case where Read task parameters from database #3 failed for some reason)

Defer #2 until we can verify the correct usage against MetaWear SDK examples — it could be the right cleanup or could introduce a new crash.

Related

• Harden shutdown path against native crashes and SSH tunnel leaks #648 (merged) — prior close() reordering and shutdown work
• docs/arch/crash_analysis_mbient_2026-04.md — crash site 4 context
• Switch to file logging before device teardown on terminate #649 — fallback file logging during device teardown (complementary)

[lwhite1]

Update (2026-04-05): Cross-machine simultaneous crash at shutdown rules out radio contention

A crash from today confirms the shutdown-path hypothesis and narrows the root cause considerably.

Evidence

Both ACQ_0 (3 mbients) and ACQ_1 (2 mbients) crashed with identical Fatal Python error: Aborted traces at the same moment, immediately after TerminateServerRequest was sent from the GUI. Crash site: warble/gattchar.py:53 _private_write_async.

ACQ_0 and ACQ_1 are on separate physical machines with separate USB Bluetooth dongles. They share no BLE infrastructure. Ruling out explanations:

• Not radio contention — independent dongles can't interfere with each other
• Not sensor hardware instability — different sensor sets on the two machines
• Not flaky timing — both fired at the same moment on the same code path
• Not high contention — ACQ_1 only has 2 mbients, same crash
• The only shared factor is the code that runs on TerminateServerRequest

Stack trace

gattchar.py:53  _private_write_async         ← ABORTS (native assertion)
gattchar.py:71  write_without_resp_async
metawear.py:442 _write_char_async            ← issues next write from queue
metawear.py:437 completed                    ← completion callback of previous write
gattchar.py:47  completed                    ← native warble thread

What the MetaWear SDK internals show

Reading mbientlab/metawear/metawear.py:_write_char_async (line 428) and mbientlab/warble/gattchar.py:_private_write_async (line 44):

MetaWear maintains its own write_queue per device. When a BLE write completes on a native warble thread, the completion callback pops the queue and recursively calls _write_char_async(True) to issue the next write from within the completion handler. This means successive BLE writes are not issued from Python's call stack — they're issued from native completion threads.

Additionally, GattChar._private_write_async (gattchar.py:50) stores the ctypes callback wrapper on self.write_handler, overwriting the previous one on every call. When our Python code issues writes rapidly (e.g., stop_acc → stop_gyro → disable_acc → disable_gyro all back-to-back inside stop()), they queue up, and the recursive completion→next-write chain on the native thread is what trips the abort inside _private_write_async.

Revised fix direction

The earlier proposal (stop → drain → unsubscribe → disable) is right in spirit but insufficient as specified. The drain has to be between each individual BLE write, not just between phases. Options:

1. Explicit sleeps between each Python call that enqueues a BLE write. Each sleep has to be long enough for the previous write's native completion to fully unwind before we enqueue the next. Fragile (BLE latency varies) but simple. Needs ~100-200ms per write based on typical BLE connection intervals.

2. Drain the metawear write_queue between calls. Not directly exposed — would require reaching into self.device_wrapper.device.write_queue which is internal SDK state.

3. Find a synchronous SDK alternative. The SDK uses write_without_resp_async everywhere; there's also write_async (with response) but it's still async. No blocking variant found in the SDK. Would need to wrap the async calls in Python with an event-based wait pattern.

4. Reduce the number of BLE writes on the shutdown path. stop() currently does 4 writes (stop acc, stop gyro, disable acc, disable gyro). If we skip disable_inertial_sampling entirely on shutdown (the device disconnects moments later anyway), we cut that to 2 writes + disconnect. Less stuff in flight, less chance of collision.

5. Rely on raw disconnect. On shutdown, we don't actually need to stop/disable the sensors at all — we're about to disconnect the device. A clean BLE disconnect forces the sensor into a safe state (idle, low-power). Skip stop() entirely in close(), just call disconnect(). This is the most aggressive but possibly the safest.

Recommendation

Option 5 (skip stop() on shutdown, just disconnect) is the most promising because it eliminates the entire write-chain race at shutdown. The downside is that sensors may continue running briefly after disconnect, but they'll enter their own disconnect-timeout behavior within seconds. Worth testing.

Fallback: Option 1 (explicit sleeps) if option 5 has unexpected side effects.

Closing out earlier assumptions

Earlier I wrote in the crash analysis doc and in comments on this repo:

These crashes cannot be prevented by a Python-level lock because the callbacks fire from within the native C library, outside of Python's control.

That was correct for mid-session instances of site 2. It is not correct for the shutdown path, where we control the cadence of BLE writes. We can avoid the crash by spacing our writes or reducing them.

[lwhite1]

Resolved across v0.73.0 (#651) and v0.75.0 (#655).

The three fixes originally proposed have been superseded by a more aggressive approach: all native MetaWear calls were removed from close().

• v0.73.0: Removed stop() (4 BLE writes that triggered warble write-chain aborts)
• v0.75.0: Removed mbl_mw_datasignal_unsubscribe() (access violations on stale signal handles)

close() now does: swap on_disconnect to no-op → clear Python state → disconnect(). No native MetaWear calls before disconnect.

Fix #3 from the original proposal (stop mbients before close_streams) is no longer critical since close() itself can't crash. The remaining mid-session reconnect crash risk is tracked in #654.