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 race condition involving engine.destroy() and ready event #199

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link

@Trott Trott commented Sep 22, 2019

The 'ready' event sets up an interval timer that is cleared in
destroy(). However, in some situations, it is possible to for the
interval to be created after destroy() has run, resulting in an
interval timer that is never cleared, and thus an app that does not
exit when it is supposed to. This change fixes the issue and adds a
test for it.

Refs: #198

The 'ready' event sets up an interval timer that is cleared in
`destroy()`. However, in some situations, it is possible to for the
interval to be created after `destroy()` has run, resulting in an
interval timer that is never cleared, and thus an app that does not
exit when it is supposed to. This change fixes the issue and adds a
test for it.

Refs: mafintosh#198
@Trott
Copy link
Author

Trott commented Oct 2, 2019

@mafintosh Sorry for the ping, but any chance this or some other equivalent change can get in soon and be released? This is a race condition that is sometimes triggered in CITGM.

@mafintosh
Copy link
Owner

It’s on my todo for tmw now

@Trott
Copy link
Author

Trott commented Oct 18, 2019

/re-ping! We're down to just three errors in CITGM on the upcoming LTS branch and this is one of them. Would love to get it fixed soon!

@mafintosh
Copy link
Owner

Thanks @Trott ! I didn't realise you actually sent a PR and not just the issue until I pushed a similar fix.

Anyways should be fixed now

@mafintosh mafintosh closed this Oct 19, 2019
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