diff --git a/lib/src/main/java/org/asamk/signal/manager/helper/CallManager.java b/lib/src/main/java/org/asamk/signal/manager/helper/CallManager.java index 77eca18c..eb767692 100644 --- a/lib/src/main/java/org/asamk/signal/manager/helper/CallManager.java +++ b/lib/src/main/java/org/asamk/signal/manager/helper/CallManager.java @@ -311,6 +311,9 @@ public class CallManager implements AutoCloseable { return; } synchronized (state) { + if (activeCalls.get(callId) != state) { + return; + } if (state.multiDeviceSignaling == null && type != null && type != HangupMessage.Type.NORMAL) { state.pendingRemoteNotifications.add(() -> handleIncomingHangup(sender, callId, senderDeviceId, type, deviceId)); return; @@ -333,7 +336,7 @@ public class CallManager implements AutoCloseable { case NEED_PERMISSION -> 4; }); message.put("deviceId", deviceId); - sendControlMessage(state, writeJson(message)); + sendRemoteControlMessage(state, message); } public void handleIncomingBusy(final RecipientId sender, final long callId, final int senderDeviceId) { @@ -348,7 +351,15 @@ public class CallManager implements AutoCloseable { var message = mapper.createObjectNode(); message.put("type", "receivedBusy"); message.put("senderDeviceId", senderDeviceId); - sendControlMessage(state, writeJson(message)); + sendRemoteControlMessage(state, message); + } + + private void sendRemoteControlMessage(CallState state, ObjectNode message) { + synchronized (state) { + if (activeCalls.get(state.callId) == state) { + sendControlMessage(state, writeJson(message)); + } + } } // --- Internal helpers --- @@ -768,13 +779,23 @@ public class CallManager implements AutoCloseable { } } + CallState removeCall(final long callId) { + var state = activeCalls.get(callId); + if (state == null) return null; + synchronized (state) { + if (!activeCalls.remove(callId, state)) return null; + state.pendingRemoteNotifications.clear(); + state.state = CallInfo.State.ENDED; + return state; + } + } + void endCall(final long callId, final String reason) { - var state = activeCalls.remove(callId); + var state = removeCall(callId); + if (state == null) return; dependencies.getAuthenticatedSignalWebSocket().removeKeepAliveToken("call" + callId); dependencies.getUnauthenticatedSignalWebSocket().removeKeepAliveToken("call" + callId); - if (state == null) return; - state.state = CallInfo.State.ENDED; fireCallEvent(state, reason); logger.debug("Call {} ended: {}", callIdUnsigned(callId), reason); diff --git a/lib/src/test/java/org/asamk/signal/manager/helper/CallManagerSignalingTest.java b/lib/src/test/java/org/asamk/signal/manager/helper/CallManagerSignalingTest.java index e43c3d9e..aeb2f20f 100644 --- a/lib/src/test/java/org/asamk/signal/manager/helper/CallManagerSignalingTest.java +++ b/lib/src/test/java/org/asamk/signal/manager/helper/CallManagerSignalingTest.java @@ -26,6 +26,10 @@ import java.io.StringWriter; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.Map; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import static org.junit.jupiter.api.Assertions.*; @@ -104,12 +108,66 @@ class CallManagerSignalingTest { @Test void doesNotReplayNotificationsAfterCallRemoval() throws Exception { manager.handleIncomingHangup(state.recipientId, state.callId, 1, HangupMessage.Type.ACCEPTED, 2); - calls.clear(); + manager.removeCall(state.callId); events("{\"type\":\"ready\",\"signalingVersion\":2}"); assertEquals("", output.toString()); assertTrue(state.pendingRemoteNotifications.isEmpty()); } + @Test + void callRemovalWaitsForInFlightNotification() throws Exception { + var entered = new CountDownLatch(1); + var release = new CountDownLatch(1); + var registeredAtWrite = new AtomicBoolean(); + var failure = new AtomicReference(); + state.controlWriter = new PrintWriter(output, true) { + @Override + public void println(String line) { + entered.countDown(); + try { + if (!release.await(5, TimeUnit.SECONDS)) throw new AssertionError("write gate timed out"); + registeredAtWrite.set(calls.containsKey(state.callId)); + super.println(line); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new AssertionError(e); + } + } + }; + manager.handleIncomingHangup(state.recipientId, state.callId, 1, HangupMessage.Type.ACCEPTED, 2); + var negotiation = new Thread(() -> { + try { + events("{\"type\":\"ready\",\"signalingVersion\":2}"); + } catch (Throwable e) { + failure.set(e); + } + }); + var cancellation = new Thread(() -> manager.removeCall(state.callId)); + negotiation.start(); + try { + assertTrue(entered.await(5, TimeUnit.SECONDS)); + cancellation.start(); + long deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(5); + while (cancellation.getState() != Thread.State.BLOCKED && cancellation.isAlive() + && System.nanoTime() < deadline) { + Thread.onSpinWait(); + } + assertEquals(Thread.State.BLOCKED, cancellation.getState()); + assertTrue(calls.containsKey(state.callId)); + } finally { + release.countDown(); + negotiation.join(5000); + cancellation.join(5000); + } + assertFalse(negotiation.isAlive()); + assertFalse(cancellation.isAlive()); + assertNull(failure.get()); + assertTrue(registeredAtWrite.get()); + assertFalse(calls.containsKey(state.callId)); + assertEquals(CallInfo.State.ENDED, state.state); + assertTrue(state.pendingRemoteNotifications.isEmpty()); + } + @Test void readyNegotiatesBeforeCallEvents() throws Exception { events("{\"type\":\"ready\",\"signalingVersion\":2}");