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

Fix detection of missing done in Defer from Defer #158

Conversation

dolmen
Copy link
Contributor

@dolmen dolmen commented Jun 8, 2023

Fix a case where a missing call to Done following Defer wasn't detected: the (weired) case where Defer is called from a defered function.

First commit is the failing test.

Second commit is the fix. It also refactors the detection by using two separate calls to TB.Cleanup: one to defer the function, one for the 'missing Done' detection.

Add a failing test for a 'missing Done' detection failure: a Defer
called from a defered function must have another matching Done.
Refactor Defer to make clearer that running the defered function is
just delegated to Cleanup.
The 'Done called' hook is now installed in its own Cleanup.

Note that the panic about missing Done call will now also happen after
the run of the last deferred function. Also it will be correctly
installed multiple times for the weired case where Defer() is called
from a deferred function.
@dolmen dolmen force-pushed the fix-detection-of-missing-Done-in-Defer-from-Defer branch from 8ec573f to 3ecf279 Compare June 10, 2023 07:06
Copy link
Owner

@frankban frankban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch and very nice solution!

@frankban frankban merged commit 6aef968 into frankban:master Jun 10, 2023
8 checks passed
@frankban
Copy link
Owner

frankban commented Aug 1, 2023

Released as v1.14.6.

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.

None yet

2 participants