gloss
gloss copied to clipboard
clj-commons
Add failing test case for decode-stream close! race condition
The issue seems to be caused by a race condition that closes dst when src is drained but the last value hasn't yet been put: https://github.com/ztellman/gloss/blob/master/src/gloss/io.clj#L156
Would be happy to attempt a fix, but was wondering if you have any advice on how to proceed.
Confirmed an issue, but I'm not sure what can be done at this time. Still investigating.
At the moment, there are two problems going on, I think.
One is that, on close, the connect-via callback f inside decode-stream is called one last time with an empty vec to force out any remaining bytes. This would result in an extra nil at the end of the stream, or in the case of the string frame here, an empty string, neither of which are correct. OTOH, removing it causes certain other tests to fail.
The other problem is the race between the multiple streams used internally to build decode-stream when closing. The callback function takes long enough that sometimes the downstream is closed before the callback writes a final value to it. Based on the docs, it seems like we could wait on the deferred returned by connect-via before closing, but that doesn't work because Manifold has inconsistent upstream connections (e.g., see clj-commons/manifold#128), so closing the dst doesn't close the upstream Callback. This might be rectifiable with WeakReferences, but if so, it may be better to add a WeakRef universally to the Downstream objects. Then we could fix issues like the previous one and clj-commons/manifold#85. Zach seemed to think it would be tricky, but the lack of it is causing several bugs.
Like I said, need to investigate further.
@ryuuseijin Your test was added, since bo-tato tackled the fix. How do you want to be thanked in the changelog? I generally list people's names, but I can use your username, if you prefer.
@ryuuseijin Your test was added, since bo-tato tackled the fix. How do you want to be thanked in the changelog? I generally list people's names, but I can use your username, if you prefer.
Thank you for fixing this, @KingMob and @bo-tato. Please use my username for the changelog, thanks!