Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add exception handling to CallbackList.invoke #175

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gene-pavlovsky
Copy link
Member

@gene-pavlovsky gene-pavlovsky commented Feb 2, 2024

to prevent this kind of shit: https://try.haxe.org/#50b2a5Ce

The main idea behind this PR:

  • Most importantly, if any callback or handler throws an exception, this shouldn't corrupt the internal implementation details of tink_core primitives (such as CallbackList), resulting in lasting breakage (as in the try-haxe sample program above).
  • "Bad" callbacks/handlers shouldn't ruin life for everyone else (other callbacks/handlers). Until this PR, you can have add one faulty unimportant callback/handler which has a bug, and it has the potential to break the whole application if it throws.

Any other similar places that could benefit from such error-proofing?

P.S. I now remember suggesting this:
#165
Having been bitten by this problem again (when a signal handler throws), I couldn't wait any longer and came up with this limited solution. I don't see any reason not to do this by default, because I think this fixes an actual library bug, but since it can be kind of a breaking change, maybe it should be on an opt-in basis (conditional compilation flag).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant