diff --git a/docs/guide/client-api.md b/docs/guide/client-api.md index d546b224..7cc4a362 100644 --- a/docs/guide/client-api.md +++ b/docs/guide/client-api.md @@ -620,7 +620,7 @@ You close what you created, and closing an owner closes what it owns: `WebRtc.close()` is the one close that waits, for up to the close timeout, which makes it fit for try-with-resources. Once it returns, every peer's `state()` is `CLOSED`; the `whenClosed` futures and `CLOSED` events follow on the peers' event threads, since waiting for them could never end when `close()` is called on an event thread. -When a peer or track does not close within the close timeout, the runtime leaves its factory undisposed rather than free what WebRTC's threads may still use. `WebRtc.closeAsync()` then fails with a `WebRtcException` naming the cause, and the error handler hears of it as well, which is how `close()` reports it. +When a peer or track does not close within the close timeout, or a track or its source could not be released, the runtime leaves its factory undisposed rather than free what WebRTC's threads may still use. A track's `closeAsync()` fails in that case too. `WebRtc.closeAsync()` then fails with a `WebRtcException` naming the cause, and the error handler hears of it as well, which is how `close()` reports it. ## Errors diff --git a/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalMediaImpl.java b/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalMediaImpl.java index 2e9d8927..c5bc0c23 100644 --- a/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalMediaImpl.java +++ b/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalMediaImpl.java @@ -57,6 +57,9 @@ final class LocalMediaImpl implements LocalMedia { private final Set tracks = Collections.newSetFromMap(new ConcurrentHashMap<>()); + /** Completed as the creations of tracks end, which the runtime waits for when it closes. */ + private final Set> creations = Collections.newSetFromMap(new ConcurrentHashMap<>()); + // Guarded by cameras. private final Map cameras = new HashMap<>(); @@ -91,31 +94,32 @@ public LocalCamera camera() { @Override public LocalCamera camera(CameraOptions options) { requireNonNull(options, "options"); - rtc.checkOpen(); - VideoDevice device = options.device; + try (Creation creation = begin()) { + VideoDevice device = options.device; - if (device == null) { - List available = devices().cameras(); + if (device == null) { + List available = devices().cameras(); - if (available.isEmpty()) { - throw new MediaException("No camera was found"); - } + if (available.isEmpty()) { + throw new MediaException("No camera was found"); + } - device = available.get(0); - } + device = available.get(0); + } - CameraOptions resolved = options.withDevice(device); - SharedCamera camera = openCamera(resolved); + CameraOptions resolved = options.withDevice(device); + SharedCamera camera = openCamera(resolved); - try { - VideoTrack track = rtc.factory.createVideoTrack(newId("camera"), camera.source); + try { + VideoTrack track = rtc.factory.createVideoTrack(newId("camera"), camera.source); - return register(new LocalCameraImpl(this, track, camera)); - } - catch (RuntimeException e) { - releaseCamera(camera); - throw new MediaException("The track of the camera " + device.getName() + " could not be created", e); + return register(new LocalCameraImpl(this, track, camera)); + } + catch (RuntimeException e) { + releaseCamera(camera); + throw new MediaException("The track of the camera " + device.getName() + " could not be created", e); + } } } @@ -133,72 +137,75 @@ public LocalAudio microphone() { @Override public LocalAudio microphone(AudioOptions options) { requireNonNull(options, "options"); - rtc.checkOpen(); - if (rtc.audioMode != AudioMode.DEVICES) { - throw new ConfigurationException("The runtime was built with AudioMode." + rtc.audioMode - + ", which sends pushed audio only. Build it with AudioMode.DEVICES to capture the microphone, " - + "or use customAudio() to push audio"); - } + try (Creation creation = begin()) { + if (rtc.audioMode != AudioMode.DEVICES) { + throw new ConfigurationException("The runtime was built with AudioMode." + rtc.audioMode + + ", which sends pushed audio only. Build it with AudioMode.DEVICES to capture the microphone, " + + "or use customAudio() to push audio"); + } - AudioTrackSource source = rtc.factory.createAudioSource(options); + AudioTrackSource source = rtc.factory.createAudioSource(options); - try { - AudioTrack track = rtc.factory.createAudioTrack(newId("microphone"), source); + try { + AudioTrack track = rtc.factory.createAudioTrack(newId("microphone"), source); - return register(new LocalAudioImpl(this, track, source::dispose)); - } - catch (RuntimeException e) { - source.dispose(); - throw new MediaException("The microphone track could not be created", e); + return register(new LocalAudioImpl(this, track, source::dispose)); + } + catch (RuntimeException e) { + source.dispose(); + throw new MediaException("The microphone track could not be created", e); + } } } @Override public LocalVideo screen(ScreenOptions options) { requireNonNull(options, "options"); - rtc.checkOpen(); - VideoDesktopSource source = new VideoDesktopSource(); + try (Creation creation = begin()) { + VideoDesktopSource source = new VideoDesktopSource(); - try { - source.setSourceId(options.source.id, options.window); - source.setFrameRate(options.frameRate); + try { + source.setSourceId(options.source.id, options.window); + source.setFrameRate(options.frameRate); - if (options.maxWidth > 0) { - source.setMaxFrameSize(options.maxWidth, options.maxHeight); - } + if (options.maxWidth > 0) { + source.setMaxFrameSize(options.maxWidth, options.maxHeight); + } - source.start(); + source.start(); - VideoTrack track = rtc.factory.createVideoTrack(newId(options.window ? "window" : "screen"), source); + VideoTrack track = rtc.factory.createVideoTrack(newId(options.window ? "window" : "screen"), source); - return register(new LocalVideoImpl(this, track, () -> { - source.stop(); + return register(new LocalVideoImpl(this, track, () -> { + source.stop(); + source.dispose(); + })); + } + catch (RuntimeException e) { source.dispose(); - })); - } - catch (RuntimeException e) { - source.dispose(); - throw new MediaException("Capturing " + options.source.title + " failed: " + e.getMessage(), e); + throw new MediaException("Capturing " + options.source.title + " failed: " + e.getMessage(), e); + } } } @Override public CustomVideo customVideo(String label) { requireNonNull(label, "label"); - rtc.checkOpen(); - CustomVideoSource source = new CustomVideoSource(); + try (Creation creation = begin()) { + CustomVideoSource source = new CustomVideoSource(); - try { - VideoTrack track = rtc.factory.createVideoTrack(label, source); + try { + VideoTrack track = rtc.factory.createVideoTrack(label, source); - return register(new CustomVideoImpl(this, track, source)); - } - catch (RuntimeException e) { - source.dispose(); - throw e; + return register(new CustomVideoImpl(this, track, source)); + } + catch (RuntimeException e) { + source.dispose(); + throw e; + } } } @@ -206,63 +213,92 @@ public CustomVideo customVideo(String label) { public LocalVideo videoTrack(String label, VideoTrackSource source) { requireNonNull(label, "label"); requireNonNull(source, "source"); - rtc.checkOpen(); - VideoTrack track = rtc.factory.createVideoTrack(label, source); + try (Creation creation = begin()) { + VideoTrack track = rtc.factory.createVideoTrack(label, source); - // The source is the caller's, and outlives the track. - return register(new LocalVideoImpl(this, track, () -> { })); + // The source is the caller's, and outlives the track. + return register(new LocalVideoImpl(this, track, () -> { })); + } } @Override public LocalAudio audioTrack(String label, AudioTrackSource source) { requireNonNull(label, "label"); requireNonNull(source, "source"); - rtc.checkOpen(); - boolean pushed = source instanceof CustomAudioSource; + try (Creation creation = begin()) { + boolean pushed = source instanceof CustomAudioSource; - if (pushed && rtc.audioMode == AudioMode.DEVICES) { - throw new ConfigurationException("A CustomAudioSource needs a runtime built with AudioMode.CUSTOM or " - + "AudioMode.HEADLESS; this one captures the microphone"); - } - if (!pushed && rtc.audioMode != AudioMode.DEVICES) { - throw new ConfigurationException("A captured audio source needs a runtime built with AudioMode.DEVICES; " - + "this one was built with AudioMode." + rtc.audioMode); - } + if (pushed && rtc.audioMode == AudioMode.DEVICES) { + throw new ConfigurationException("A CustomAudioSource needs a runtime built with AudioMode.CUSTOM or " + + "AudioMode.HEADLESS; this one captures the microphone"); + } + if (!pushed && rtc.audioMode != AudioMode.DEVICES) { + throw new ConfigurationException("A captured audio source needs a runtime built with AudioMode.DEVICES; " + + "this one was built with AudioMode." + rtc.audioMode); + } - AudioTrack track = rtc.factory.createAudioTrack(label, source); + AudioTrack track = rtc.factory.createAudioTrack(label, source); - // The source is the caller's, and outlives the track. - return register(new LocalAudioImpl(this, track, () -> { })); + // The source is the caller's, and outlives the track. + return register(new LocalAudioImpl(this, track, () -> { })); + } } @Override public CustomAudio customAudio(String label) { requireNonNull(label, "label"); - rtc.checkOpen(); - if (rtc.audioMode == AudioMode.DEVICES) { - throw new ConfigurationException("The runtime was built with AudioMode.DEVICES, which sends the " - + "microphone. WebRTC feeds captured audio into every audio sender of a runtime, so pushed " - + "audio needs a runtime built with AudioMode.CUSTOM or AudioMode.HEADLESS"); - } + try (Creation creation = begin()) { + if (rtc.audioMode == AudioMode.DEVICES) { + throw new ConfigurationException("The runtime was built with AudioMode.DEVICES, which sends the " + + "microphone. WebRTC feeds captured audio into every audio sender of a runtime, so pushed " + + "audio needs a runtime built with AudioMode.CUSTOM or AudioMode.HEADLESS"); + } - CustomAudioSource source = new CustomAudioSource(); + CustomAudioSource source = new CustomAudioSource(); - try { - AudioTrack track = rtc.factory.createAudioTrack(label, source); + try { + AudioTrack track = rtc.factory.createAudioTrack(label, source); - return register(new CustomAudioImpl(this, track, source)); + return register(new CustomAudioImpl(this, track, source)); + } + catch (RuntimeException e) { + source.dispose(); + throw e; + } } - catch (RuntimeException e) { - source.dispose(); - throw e; + } + + /** + * Begins to create a track, before anything native is touched. The + * creation is registered before the runtime is checked again: a runtime + * that begins to close either waits for the creation before it frees the + * factory, or is seen closing here. + * + * @return the creation, to close once the track is registered or failed. + */ + Creation begin() { + Creation creation = new Creation(); + creations.add(creation.done); + + if (rtc.isClosing()) { + creation.close(); + throw new PeerClosedException("The runtime is closed"); } + + return creation; } + /** + * Closes every track, and names the creations that still run. Those are + * listed first: a creation registers its track before it ends, so a track + * that one makes meanwhile is either listed or comes from a creation + * listed, and the runtime closes it in its next round. + */ List> closeAll() { - List> closing = new ArrayList<>(); + List> closing = new ArrayList<>(creations); for (LocalTrackImpl track : new ArrayList<>(tracks)) { closing.add(track.closeInternal()); @@ -282,14 +318,8 @@ void forget(LocalTrackImpl track) { tracks.remove(track); } - private T register(T track) { + T register(T track) { tracks.add(track); - - if (rtc.isClosing()) { - // The runtime began to close meanwhile and may have missed it. - track.closeInternal(); - } - return track; } @@ -390,6 +420,21 @@ private static String newId(String kind) { + /** A creation of a track, which the runtime waits for when it closes. */ + final class Creation implements AutoCloseable { + + final CompletableFuture done = new CompletableFuture<>(); + + + @Override + public void close() { + creations.remove(done); + done.complete(null); + } + } + + + static final class SharedCamera { final String key; diff --git a/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalTrack.java b/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalTrack.java index 9ea63ded..292e0582 100644 --- a/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalTrack.java +++ b/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalTrack.java @@ -68,7 +68,9 @@ public interface LocalTrack extends AutoCloseable { * * @return a future that completes once the native track is released, after * which a source the track was made over, with {@link - * LocalMedia#videoTrack} or {@link LocalMedia#audioTrack}, may be closed. + * LocalMedia#videoTrack} or {@link LocalMedia#audioTrack}, may be closed; + * or fails with a {@link WebRtcException} when the track or its source + * could not be released, which then fails the runtime's close as well. */ CompletableFuture closeAsync(); diff --git a/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalTrackImpl.java b/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalTrackImpl.java index 90db93d3..552d529e 100644 --- a/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalTrackImpl.java +++ b/webrtc/src/main/java/dev/onvoid/webrtc/client/LocalTrackImpl.java @@ -218,26 +218,30 @@ void beforeRelease() { } private void release() { - try { - beforeRelease(); + MediaStreamTrack last; + Runnable lastRelease; - MediaStreamTrack last; - Runnable lastRelease; + synchronized (this) { + last = track; + lastRelease = releaseSource; + } - synchronized (this) { - last = track; - lastRelease = releaseSource; - } + // Each step is tried, whether or not the one before failed: the source + // holds a reference of its own and is released even when the track is not. + Throwable failure = attempt(null, this::beforeRelease); + failure = attempt(failure, last::dispose); + failure = attempt(failure, lastRelease); - last.dispose(); - lastRelease.run(); + if (failure == null) { + media.forget(this); } - catch (Throwable e) { - rtc.reportError(e); + else { + // Kept among the tracks, so that the runtime, which closes them + // again, sees what was left and leaves the factory undisposed. + failure = new WebRtcException("Releasing " + this + " failed", failure); + rtc.reportError(failure); } - media.forget(this); - List handlers; synchronized (this) { @@ -255,7 +259,32 @@ private void release() { } } - closed.complete(null); + if (failure == null) { + closed.complete(null); + } + else { + closed.completeExceptionally(failure); + } + } + + /** + * Runs a step of the release. + * + * @return the failure so far, with the step's added to it. + */ + private static Throwable attempt(Throwable failure, Runnable step) { + try { + step.run(); + } + catch (Throwable e) { + if (failure == null) { + return e; + } + + failure.addSuppressed(e); + } + + return failure; } diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/client/RuntimeCloseTests.java b/webrtc/src/test/java/dev/onvoid/webrtc/client/RuntimeCloseTests.java index 940dd68a..cd6a3b66 100644 --- a/webrtc/src/test/java/dev/onvoid/webrtc/client/RuntimeCloseTests.java +++ b/webrtc/src/test/java/dev/onvoid/webrtc/client/RuntimeCloseTests.java @@ -28,6 +28,9 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +import dev.onvoid.webrtc.media.video.CustomVideoSource; import org.junit.jupiter.api.Test; @@ -231,6 +234,91 @@ void closeFailureReportedOnce() throws Exception { } } + @Test + void closeBeforeTrackCreated() throws Exception { + CompletableFuture closed = rtc.closeAsync(); + + assertThrows(PeerClosedException.class, () -> rtc.media().customVideo("late")); + await(closed); + } + + @Test + void closeWhileCreatingTrack() throws Exception { + // The runtime begins to close while a track is created over its + // factory, which it keeps until the track is made, and then closed. + ManualExecutor control = new ManualExecutor(); + WebRtc runtime = runtime(control); + LocalMediaImpl media = (LocalMediaImpl) runtime.media(); + CompletableFuture closed; + CustomVideoImpl track; + + try (LocalMediaImpl.Creation creation = media.begin()) { + closed = runtime.closeAsync(); + + CustomVideoSource source = new CustomVideoSource(); + track = media.register(new CustomVideoImpl(media, runtime.factory.createVideoTrack("late", source), + source)); + } + + // The release of the track, which the runtime waits for. + control.awaitQueued(TIMEOUT_SECONDS); + assertTrue(track.isEnded()); + assertFalse(closed.isDone()); + + control.runUntil(closed::isDone, TIMEOUT_SECONDS); + await(closed); + await(track.closeAsync()); + } + + @Test + void releaseFailureFailsClose() throws Exception { + // A step of the release fails. The steps after it run all the same, + // and the track's close fails, as does the runtime's, which leaves + // the factory undisposed. + WebRtc runtime = WebRtc.builder() + .audio(AudioMode.HEADLESS) + .errorHandler(errors::add) + .build(); + LocalMediaImpl media = (LocalMediaImpl) runtime.media(); + IllegalStateException broken = new IllegalStateException("Broken"); + AtomicBoolean sourceReleased = new AtomicBoolean(); + LocalVideoImpl track; + + try (LocalMediaImpl.Creation creation = media.begin()) { + CustomVideoSource source = new CustomVideoSource(); + Runnable release = () -> { + source.dispose(); + sourceReleased.set(true); + }; + + track = media.register(new LocalVideoImpl(media, runtime.factory.createVideoTrack("broken", source), + release) { + + @Override + void beforeRelease() { + throw broken; + } + }); + } + + try { + ExecutionException e = assertThrows(ExecutionException.class, () -> await(track.closeAsync())); + assertSame(broken, e.getCause().getCause()); + assertTrue(sourceReleased.get()); + assertTrue(track.isEnded()); + + ExecutionException closing = assertThrows(ExecutionException.class, () -> await(runtime.closeAsync())); + assertTrue(closing.getCause().getMessage().contains("left undisposed"), closing.getCause()::getMessage); + + assertEquals(2, errors.size(), () -> "Errors: " + errors); + assertSame(e.getCause(), errors.get(0)); + assertSame(closing.getCause(), errors.get(1)); + } + finally { + errors.clear(); + } + } + private WebRtc runtime(ManualExecutor control) { return WebRtc.builder() .audio(AudioMode.HEADLESS)