Wake takers parked in MessageQueue.take() when a read fails

[steviec]

Problem

Two threads sharing one MessageQueue (e.g. a keep-alive reader thread + a request thread over one ADB connection) could hang forever. In take(), signalAll() ran only after a successful read(). When read() threw (connection reset / EOF / SocketTimeoutException), any taker parked in queueCond.await() was never woken — a classic lost wakeup. Neither the read nor the write timeout covers await().

Fix

• Signal on every read() outcome (try { read() } finally { signalAll() }). A failed read releases readLock and wakes the waiters; each woken taker retries the read and observes the failure for itself. Errors stay per-call and non-sticky — a transient read error no longer poisons the connection.
• close() is the one terminal case. It sets a close marker (signalClosed) and wakes every parked taker so they fail fast with a clean IOException, instead of stranding in await() or waking to re-read an already-closed reader (which would surface as IllegalStateException). The marker is set only on close — when the socket also closes — so it stays in step with DadbImpl rebuilding the connection on the next op (no permanent bricking).

This preserves the pre-existing failure contract (per-call, retryable IOException; connection liveness owned by the socket/DadbImpl) and only closes the lost-wakeup hole.

Tests

Deterministic and emulator-free:

• a read failure wakes every parked taker, each as IOException;
• a transient read failure does not poison the queue — a later take() still delivers a subsequently-read message;
• close() wakes every parked taker with a clean IOException (exercised through the real AdbMessageQueue/AdbReader path).

Existing happy-path concurrency tests (MessageQueueTest) continue to pass.

[amanjeetsingh150]

Just writing one concern a bigger question we can chat offline about: How should we get confidence on changes with dadb? e2e on maestro core should help here but we might want to do snapshot release and then do final one.

[steviec]

Just writing one concern a bigger question we can chat offline about: How should we get confidence on changes with dadb? e2e on maestro core should help here but we might want to do snapshot release and then do final one.

I had the exact same question as I was making these changes. The problem with this (and with maestro core) is lack of robust test coverage. If we already have a harness in place that we had confidence really tested the mechanics of this library, we could make changes rapidly with confidence. So the real fix, IMO, would be to actually rebuild this library from the ground up, driven by behavior tests that are rock solid. Tests and test harnesses are more important than code now IMO, so if you have code without tests, the code is potentially not worth keeping (and only using as guidance for "correct" behavior to the test harness that you're building)