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

Deadlock due to race in FsEventWatcher #118

Closed
michaelfairley opened this issue May 16, 2017 · 5 comments
Closed

Deadlock due to race in FsEventWatcher #118

michaelfairley opened this issue May 16, 2017 · 5 comments
Labels
A-bug os-mac Z-needs implementation Needs an implementation, will accept PRs

Comments

@michaelfairley
Copy link
Contributor

Because the runloop thread signals that the runloop has started before CFRunLoopStart is called, it's possible for CFRunLoopStop to get called before CFRunLoopStart, causing the runloop to never stop and thus the done channel to be waited on forever.

In practice, if I have two watch commands back to back, I get a deadlock roughly 10% of the time on my Mac.

I have a somewhat cheesy workaround that I'm using right now, but an ideal solution probably involves CFRunLoopObserver or somesuch.

@passcod passcod added A-bug Z-needs implementation Needs an implementation, will accept PRs os-mac labels May 16, 2017
@cmyr
Copy link
Contributor

cmyr commented Nov 22, 2017

I think this bug is serious enough that the proposed patch should be accepted. I'm not sure it's worth making major changes to FsEventWatcher when #117 will probably require another one. @passcod do you have any thoughts?

@passcod
Copy link
Member

passcod commented Nov 22, 2017

@michaelfairley are you okay with me using your workaround?

@michaelfairley
Copy link
Contributor Author

Yes! Please do! It's been working well enough for me since I initially filed this issues.

@cmyr
Copy link
Contributor

cmyr commented Nov 22, 2017

@michaelfairley are you in a position to PR your patch? If not I can, save @passcod the time.

@passcod
Copy link
Member

passcod commented Nov 22, 2017

Done with #134

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-bug os-mac Z-needs implementation Needs an implementation, will accept PRs
Projects
None yet
Development

No branches or pull requests

3 participants