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

errorVariable and thread safety #45

Closed
elcritch opened this issue Aug 28, 2023 · 1 comment · Fixed by #46
Closed

errorVariable and thread safety #45

elcritch opened this issue Aug 28, 2023 · 1 comment · Fixed by #46

Comments

@elcritch
Copy link
Contributor

While digging into #41 I noticed that binderror uses a global to store the last error state:

https://github.com/codex-storage/questionable/blob/e56cf86c4a089c78a1b7c3005f13343bfbbe3b48/questionable/private/binderror.nim#L4C10-L4C10

It should probably be made into var errorVariable: ptr ref CatchableError.

If it's not a threadvar issues might arise when used with multiple threads. I haven't worked through the precise logic, but some sequence similar to:

  • thread1 and thread1 both encounter errors in a without clause
  • thread1 uses errorVariable to store an error ptr
  • thread1 is paused and thread2 runs
  • thread2 stores its error in errorVariable
  • thread2 is paused
  • thread1 now reads errorVariable and gets the error meant for thread2

Probably not all too likely and shouldn't cause a false error case, just report the wrong error. That'd still be annoying.

@markspanbroek
Copy link
Member

Good find, thanks! I was able to reproduce this in a test and adding {.threadvar.} indeed fixes the problem: #46

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 a pull request may close this issue.

2 participants