Skip to content

log and guard the sender unlock in BaseSender.stop - #2198

Open
RomanHerbstmann wants to merge 1 commit into
pedroSG94:masterfrom
RomanHerbstmann:fix/log-sender-unlock-on-stop
Open

RomanHerbstmann wants to merge 1 commit into
pedroSG94:masterfrom
RomanHerbstmann:fix/log-sender-unlock-on-stop

Conversation

@RomanHerbstmann

Copy link
Copy Markdown
Contributor

Log and guard the sender unlock in BaseSender.stop

Small follow-up to #2195 / #2197.

Problem

Since the unlock moved into BaseSender.stop(clear, unlockNeeded):

  • No log line anymore when the unlock path is taken. When a sender does not stop within the timeout, the socket is closed to unblock a write stuck on TCP backpressure. Previously RtmpClient/RtspClient logged sender blocked in socket write at that point. Without it, logcat no longer shows whether a slow disconnect or reconnect was caused by a blocked write, which makes field issues hard to diagnose.
  • An exception from unlockNeeded() propagates out of stop() and disconnect(). The current callers pass socket?.close(), which does not throw in practice, but unlockNeeded is a public parameter and a throwing lambda would escape into the reconnect scope.

Fix

  • Log.w with the sender's TAG right before calling unlockNeeded().
  • unlockNeeded() is wrapped in try/catch: CancellationException is rethrown so structured cancellation keeps working, and any other exception is logged instead of escaping. runCatching is intentionally not used, because it would also swallow CancellationException.
  • No other behaviour change: same timeouts, same order of operations, same signature.

Test

  • Two new unit tests in BaseSenderTest:
    • A sender whose job ignores cancellation until unlockNeeded releases it: unlockNeeded is invoked and stop() returns. This adds coverage for the unlock path, which had no test yet.
    • unlockNeeded throws: the exception does not propagate out of stop(). This test fails without this change.
  • ./gradlew :common:testDebugUnitTest: all tests pass.
  • The unlock path itself was verified with rtmp/rtsp: unblock a sender stuck in a socket write on disconnect #2195 on an Android emulator streaming RTMP to MediaMTX under TCP backpressure (iptables -A INPUT -p tcp --sport 1935 -j DROP). This PR only adds logging and exception handling around it.

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

1 participant