Skip to content

fix(droidplug): Take QueueStream items under the lock in pollNext - #497

Merged
qdot merged 2 commits into
deviceplug:devfrom
LucaCappelletti94:upstream/queuestream-remove-under-lock
Oct 10, 2026
Merged

qdot merged 2 commits into
deviceplug:devfrom
LucaCappelletti94:upstream/queuestream-remove-under-lock

Conversation

@LucaCappelletti94

Copy link
Copy Markdown
Contributor

This is one of those changes that are super-small and super-long to explain.

Problem

On Android, btleplug can lose notifications, or stop delivering them while a thread spins at full CPU.

Seen on devices

When a device (either a linux machine, or another phone) streamed numbered notifications to btleplug on a Galaxy A35 (Android 15) and a Galaxy M52 (Android 13). With the current code, notifications() stopped delivering within the first 20,000 on both, and the polling thread spun at full CPU. Its profile shows LinkedList.removeFirst throwing NoSuchElementException on every poll, and the filter_map in notifications() dropping each error and polling again.

Cause

In the old line the remove() sits inside the inner lambda, so it runs only when Rust calls StreamPoll.get(), after pollNext has released this.lock. this.result is the queue, so that remove() edits the LinkedList without the lock while the Binder callback adds under the lock. A collision loses a notification, or leaves a non-zero size with no nodes, after which every remove() throws.

deviceplug/jni-utils-rs#3 moved add under the lock for #311 but left this remove outside.

sequenceDiagram
    participant R as Rust poll thread
    participant Q as QueueStream
    participant B as Binder thread
    R->>Q: pollNext(), lock held
    Q-->>R: ready, item left in queue
    Note over Q: lock released
    B->>Q: add(n2), lock held
    R->>Q: get() calls remove(), no lock
    Note over Q,B: add and remove race, n2 can be lost
Loading

Fix

The new lines call remove() inside the synchronized block and the lambda only hands back the taken item, so every queue access is locked.

sequenceDiagram
    participant R as Rust poll thread
    participant Q as QueueStream
    participant B as Binder thread
    R->>Q: pollNext(), lock held
    Note over Q: remove() under lock
    Q-->>R: ready, StreamPoll holds item
    B->>Q: add(n2), lock held
    Note over R: get() returns the item
Loading

An item now leaves the queue when pollNext reports ready, not when the result is read, so a ready result that is never read loses its item. The only caller, JSendStream, always reads it at once. The stress test reproduces the loss on the host JVM.

With this PR both phones received 5 × 20,000 notifications in order.

@qdot
qdot changed the base branch from master to dev October 10, 2026 21:25
@qdot
qdot merged commit 16e03b4 into deviceplug:dev Oct 10, 2026
3 of 5 checks passed
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.

2 participants