Notify TransformationListener before releasing TransformationJob #198
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I have seen the release() call hang in certain cases when processing failed as a result of which the listener callback never invoked which caused our internal logic to get stuck as well. I think it's better to first invoke the callback so that LiTr users can at least somehow propagate the error to end users or log the associated exception.
|
Fixes #197 |
izzytwosheds
approved these changes
Jun 28, 2022
|
Thank you for the fix and careful investigation. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I have seen the
release()call hang in certain cases when processing failed as a result of which the listenercallback never invoked. Which then broke our internal app logic as the processing seemed stuck forever.
I think it's better to first invoke the callback so that LiTr users can at least somehow propagate the error to end users or log the associated exception.
How to test:
This video shows a recording of the issue in Android Studio. I set a breakpoint to manifest that the listener is never hit when it gets stuck in the release call.
https://youtu.be/vo-QVk0agJE