SodiumFRP / sodium

Sodium - Functional Reactive Programming (FRP) Library for multiple languages
http://sodium.nz/
Other
848 stars 138 forks source link

Java: Transaction fails to close if exception is thrown. #149

Closed greyson closed 6 years ago

greyson commented 6 years ago

While I understand that the operations between events should not throw exceptions in a perfect implementation, previous versions would log the exception, abort the transaction, but allow new transactions to be handled. With the newer versions, if there is a single RuntimeException, my application ceases to work entirely, as it will no longer accept send on any sink.

Having used sodium for numerous android applications; and now using it in a high performance, high throughput Swing application with the current master it's been even more difficult to debug than usual (one side effect of this is that every unit test run after an unrelated unit test causes a transaction to remain open will also fail because no events will propagate).

the-real-blackh commented 6 years ago

This is definitely a bug. I can't clearly see what the cause is, though. The code is using 'finally' to clean up the transaction. Is it possible for you to write a test case?

jam40jeff commented 6 years ago

@the-real-blackh I ran into this bug in the C# version a while ago. I should have opened an issue when I fixed it.

The problem is when an exception happens during the call to close(), currentTransaction doesn't get set back to null.

the-real-blackh commented 6 years ago

Of course! Hi @greyson, I checked in a fix 9d3b64844cd151e09c6db83129a974f4fc3b265b for this. Are you in a position to re-test?

greyson commented 6 years ago

Hi @the-real-blackh,

Unfortunately, it's not happening in my current tests. I haven't been able to reproduce it outside of my existing infrastructure (which is now quite large) and was only able to put an hour into trying today (deadline on the 15th is looming!).

I'll put some more effort into figuring out what it is about my architecture that causes this; as an explaination, I have a subclass of StreamSink which diverts sends to a dedicated logic thread, and each UI listener then uses invokeLater on the AWT thread to perform operations -- Also some stream abstractions to move particularly long number crunching (which is nonetheless idempotent) off to a thread pool either interruptibly or ignorably (depending on how important it is to use the most recent data)

In other words, there's a great deal of untangling to be done to figure out why, exactly, transactions stopped closing; but it always started with a DivZero. I'll refrain from pulling the latest master for the time being, in the hopes that it will reproduce sometime today.

greyson commented 6 years ago

Alright. It did, in fact, reproduce organically today resulting from CellLoop sampled before looped; An expected error for this test, since I haven't completed implementation.

9d3b648 Fixes the issue.

greyson commented 6 years ago

Looking through a few other closed issues, I see that the reporter is closing the issue; If you would prefer to close them yourself in the future, just let me know.

the-real-blackh commented 6 years ago

@greyson That's fine! Thank you for that. I've got a few changes to make, then I'll put out a new official release soon with this bug fix in it.