Summary
#983 fixes ScreenCaptureService staying bound when a coroutine is cancelled during ScreenCaptureConnection.connect(). A broader follow-up remains: LocalParticipant.setScreenShareEnabled / setTrackEnabled can still abandon a newly created screencast track on cancellation without calling stop() / dispose(), leaking WebRTC and capture resources. If cancel happens after a successful bind, the service binding is orphaned too.
Background
Screen share enable currently does roughly:
val track = createScreencastTrack(...)
track.startForegroundService(...) // suspend: bind + startForeground
track.startCapture()
if (!publishVideoTrack(track, ...)) {
// cleanup only on publish failure
track.stopCapture()
track.stop()
track.dispose()
}setTrackEnabled has no try / finally around this path. Cleanup runs only when publishVideoTrack returns false. A CancellationException from startForegroundService or publishVideoTrack unwinds out of the function and drops the local track reference without cleanup.
After startForegroundService returns, only LocalScreencastVideoTrack.stop() → ScreenCaptureConnection.stop() releases the binding. Cancel after that point therefore also leaves ScreenCaptureService bound for the lifetime of the context.
What gets orphaned
| Cancellation timing |
Service binding |
Other resources |
After createScreencastTrack, during connect() (before it returns) |
Released by #983 |
Track / SurfaceTextureHelper / VideoSource / capturer / rtcTrack orphaned |
After startForegroundService returns, before / during publishVideoTrack |
Orphaned |
Same as above; after startCapture(), active MediaProjection can keep running too |
Expected behavior
Cancelling setScreenShareEnabled(true, ...) (or the surrounding scope) should always tear down any screencast track that was created but not successfully published: stop capture, unbind / stop the screen capture service if it was started, dispose WebRTC objects, and invoke any onStop callback as appropriate.
Suggested direction
- Make ownership transfer cancellation-safe in
LocalParticipant.setTrackEnabled for SCREEN_SHARE (and likely review the camera / mic enable paths for the same pattern): e.g. try / finally that disposes the unpublished track on cancel or failure.
- Confirm
LocalScreencastVideoTrack / SurfaceTextureHelper ownership so dispose is correct even when publish never completes.
- Add tests that cancel at each suspend point (
startForegroundService, publishVideoTrack) and assert resources are released (and the service is unbound when a bind had succeeded).
Related
Summary
#983 fixes
ScreenCaptureServicestaying bound when a coroutine is cancelled duringScreenCaptureConnection.connect(). A broader follow-up remains:LocalParticipant.setScreenShareEnabled/setTrackEnabledcan still abandon a newly created screencast track on cancellation without callingstop()/dispose(), leaking WebRTC and capture resources. If cancel happens after a successful bind, the service binding is orphaned too.Background
Screen share enable currently does roughly:
setTrackEnabledhas notry/finallyaround this path. Cleanup runs only whenpublishVideoTrackreturnsfalse. ACancellationExceptionfromstartForegroundServiceorpublishVideoTrackunwinds out of the function and drops the localtrackreference without cleanup.After
startForegroundServicereturns, onlyLocalScreencastVideoTrack.stop()→ScreenCaptureConnection.stop()releases the binding. Cancel after that point therefore also leavesScreenCaptureServicebound for the lifetime of the context.What gets orphaned
createScreencastTrack, duringconnect()(before it returns)SurfaceTextureHelper/VideoSource/ capturer / rtcTrack orphanedstartForegroundServicereturns, before / duringpublishVideoTrackstartCapture(), activeMediaProjectioncan keep running tooExpected behavior
Cancelling
setScreenShareEnabled(true, ...)(or the surrounding scope) should always tear down any screencast track that was created but not successfully published: stop capture, unbind / stop the screen capture service if it was started, dispose WebRTC objects, and invoke anyonStopcallback as appropriate.Suggested direction
LocalParticipant.setTrackEnabledforSCREEN_SHARE(and likely review the camera / mic enable paths for the same pattern): e.g.try/finallythat disposes the unpublished track on cancel or failure.LocalScreencastVideoTrack/SurfaceTextureHelperownership so dispose is correct even when publish never completes.startForegroundService,publishVideoTrack) and assert resources are released (and the service is unbound when a bind had succeeded).Related
connect(); explicitly deferred this follow-up)