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.

Sign up for GitHub

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

Jump to bottom

Notifier bugs #959

Closed
virtuald opened this issue Mar 2, 2018 · 2 comments
Closed

Notifier bugs #959

virtuald opened this issue Mar 2, 2018 · 2 comments

Comments

@virtuald
Copy link
Member

virtuald commented Mar 2, 2018 •
edited
Loading

There are two bugs I've found:

  • There's a race condition when creating/cleaning up notifiers on more than one thread. Because HAL is only using an atomic variable to guard the notifier manager cleanup, it's possible to disable all notifiers after creating a new one (but before it's fully initialized?).
  • When cleaning up the last notifier, it takes up to a second to clean it up.

A workaround for this issue is to just create a notifier handle at startup and throw it away.
virtuald added a commit that referenced this issue Mar 2, 2018
@virtuald
HAL Notifier: Don't disable the notifier manager when the last handle…
59fdc8c 
… is cleaned up

- Addresses #959, but not a good long term solution
@virtuald virtuald mentioned this issue Mar 2, 2018
Merged
virtuald added a commit that referenced this issue Mar 2, 2018
@virtuald
HAL Notifier: Don't disable the notifier manager when the last handle…
be1826c 
… is cleaned up

- Addresses #959, but not a good long term solution
virtuald added a commit that referenced this issue Mar 2, 2018
@virtuald
HAL Notifier: Don't disable the notifier manager when the last handle…
8b78c4c 
… is cleaned up

- Addresses #959, but not a good long term solution
PeterJohnson pushed a commit that referenced this issue Mar 3, 2018
@virtuald @PeterJohnson
HAL Notifier: Don't disable the notifier manager when the last handle…
1e7d439 
… is cleaned up (#960)

- Addresses #959, but not a good long term solution
@AustinShalit
Copy link
Member

Is this still an issue?

@virtuald
Copy link
Member Author

virtuald commented Aug 7, 2019

Probably not?

@virtuald virtuald closed this as completed Aug 7, 2019
pjbuterbaugh pushed a commit to pjbuterbaugh/allwpilib that referenced this issue Jun 15, 2023
@TheTripleV @AustinShalit @Daltz333
Link to local api docs in installer included docs (wpilibsuite#959)
537fb9c 
Co-authored-by: Austin Shalit <austinshalit@gmail.com>
Co-authored-by: Daltz333 <daltzsmith@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@virtuald @AustinShalit