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

Avoid race condition while starting coqtail #366

Merged
merged 1 commit into from
Aug 23, 2024
Merged

Conversation

whonore
Copy link
Owner

@whonore whonore commented Aug 23, 2024

As described in #364, now that coqtail#start calls start asynchronously, calling a command during CoqStart (e.g., :CoqToLine | CoqToLine) can cause a race that often results in a fatal error. The solution implemented here is to have coqtail#start check if Coqtail is initialized (s:initted()) but not yet running (s:running()) and, if so, drop the command with a warning. The only difference between s:initted() and s:running() is whether b:coqtail_started is set, so I believe this should ensure that no other commands can run until coqtail#after_startCB completes. Since I expect it should be pretty rare in practice for someone to try to run commands before CoqStart completes, I think just ignoring them is reasonable. This should not affect commands that are called before CoqStart, those will still be queued to run by coqtail#after_startCB.

Fixes #364.

@whonore
Copy link
Owner Author

whonore commented Aug 23, 2024

@tomtomjhj what do you think? This seems to fix the issue on my end.

As described in #364, now that `coqtail#start` calls `start` asynchronously,
calling a command during `CoqStart` (e.g., `:CoqToLine | CoqToLine`) can cause
a race that often results in a fatal error. The solution implemented here is to
have `coqtail#start` check if Coqtail is initialized (`s:initted()`) but not
yet running (`s:running()`) and, if so, drop the command with a warning. The
only difference between `s:initted()` and `s:running()` is whether
`b:coqtail_started` is set, so I believe this should ensure that no other
commands can run until `coqtail#after_startCB` completes. Since I expect it
should be pretty rare in practice for someone to try to run commands before
`CoqStart` completes, I think just ignoring them is reasonable. This should not
affect commands that are called before `CoqStart`, those will still be queued
to run by `coqtail#after_startCB`.

Fixes #364.
@whonore whonore force-pushed the start-race-condition branch from b50e57b to 77f55bc Compare August 23, 2024 04:01
@tomtomjhj
Copy link
Contributor

I can confirm that this fixes the issue. Thanks!

@whonore whonore merged commit a3d2830 into main Aug 23, 2024
6 checks passed
@whonore whonore deleted the start-race-condition branch August 23, 2024 13:21
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.

race between coqtail#start and CoqToLine
2 participants