-
Notifications
You must be signed in to change notification settings - Fork 52
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 mingw timeout error #461
Conversation
I missed the MinGW case in this code. Sorry! I don't think using the_timeout makes sense on MinGW if thread-safe. When not thread-safe, the_timeout works -- but it's inherently wrong to use it when threaded. (We could put it in thread-local storage, but there's no reason that creation of a timeout and use of a timeout have to happen in the same thread.) IIUC, your patch would define the_timeout even when So, I think the first hunk of the patch is correct (avoid the pthread stuff on MinGW), but the second hunk shouldn't be necessary. Right now, we don't have a thread-safe version of timeouts for MinGW, so we should add:
Does that make sense? |
This will make the compilation of yices fail under MinGW when using THREAD_SAFE. However, we have external users who use THREAD_SAFE Yices under windows. I don't know if that should have been allowed at the first place. |
The answer to "should it have been allowed?" depends on how dogmatic we want to be. No platform was actually thread-safe when using timeouts; the timeouts weren't per-thread. But, since we have Windows users relying on THREAD_SAFE, then, IMO, we should fix thread-safe timeouts to work on Windows. I'm happy to do that, if you like -- though I need to figure out how to get access to a Windows box. In the meantime, we could define |
I updated the code with your suggestion. Thanks! I will email you about the access to the windows machine that I am using. |
Looks good! |
fix pthread issue for ming
the_timeout variable is used by the mingw implementation, which is not declared if THREAD_SAFE is not defined. However, it is needed for mingw. (this was changed in the recent PR about thread safety)