diff options
author | Greg <[email protected]> | 2016-12-12 17:54:22 -0800 |
---|---|---|
committer | Greg <[email protected]> | 2016-12-12 17:54:22 -0800 |
commit | b801f7cad5c1600656ca9205452b01e6a8b192a4 (patch) | |
tree | f053c11bb8be905e81e5611297c9d09b6eb525a5 /tests | |
parent | 5aa54887b7bffe4de233474e9687a63f8ae84eb7 (diff) |
Fix deadlock in AudioFifo.
The deadlock scenario is the following:
1) AudioFifo is used with wait on write and read
2) Producer Thread calls write(). The buffer is full, then it gets into
the while(available() == buffer.length) loop
3) a context switch happens right there, BEFORE readSemaphore.wait()
4) Consumer Thread calls read() multiple times until it depletes the
buffer, then ends up in writeSemaphore.wait()
5) context switch back to the Producer Thread, which now calls
readSemaphore.wait()
Deadlock: the buffer is empty, and nobody is going to signal the
producer that there is availability.
This can be reproduced with a simple stress test. I added the stress
test which is simply a copy of an existing one, with a very large value
for the chunk variable.
The race condition is more likely to be hit when the buffer is small,
but I have hit it in "production" with 32K buffers, while generating
large files (a few hundred megabytes).
I am not sure the performance implications of the change, as my use
cases are non-realtime.
Still, all the tests pass.
Diffstat (limited to 'tests')
-rw-r--r-- | tests/com/jsyn/engine/TestFifo.java | 31 |
1 files changed, 31 insertions, 0 deletions
diff --git a/tests/com/jsyn/engine/TestFifo.java b/tests/com/jsyn/engine/TestFifo.java index e504a0b..cc539d1 100644 --- a/tests/com/jsyn/engine/TestFifo.java +++ b/tests/com/jsyn/engine/TestFifo.java @@ -216,4 +216,35 @@ public class TestFifo extends TestCase { watchdog.interrupt(); } + + public void testBlockReadAndWriteWaitStress() { + final int chunk = 10000000; // 10 Megabytes + final AudioFifo fifo = new AudioFifo(); + fifo.allocate(8); + + fifo.setWriteWaitEnabled(true); + fifo.setReadWaitEnabled(true); + final double value = 50.0; + + // Schedule a delayed write in another thread. + new Thread() { + @Override + public void run() { + try { + sleep(200); + for (int i = 0; i < chunk; i++) { + fifo.write(value + i); + } + } catch (InterruptedException e) { + e.printStackTrace(); + } + } + }.start(); + + Thread watchdog = startWatchdog(10000); + for (int i = 0; i < chunk; i++) { + assertEquals("reading back data", value + i, fifo.read()); + } + watchdog.interrupt(); + } } |