-
Notifications
You must be signed in to change notification settings - Fork 212
C++11 std mutex and condition variable #1298
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
base: master
Are you sure you want to change the base?
Conversation
|
|
||
|
|
||
|
|
||
| Dynamic __hxcpp_mutex_create() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we omit these for hxcpp_api_level >= 500? It would be good to deprecate them anyway since identifiers containing __ are reserved by c++.
| } | ||
|
|
||
|
|
||
| HxMutex mMutex; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the mutex no longer uses HxMutex, I think we should also remove the HxMutex type from everywhere else in the codebase for internal consistency.
That also officially breaks the HX_THREAD_H_OVERRIDE feature, so we would have to remove that too, or continue to have a HxMutex wrapper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think replacing uses of HxMutex with std::recursive_mutex makes sense. As a replacement for HX_THREAD_H_OVERRIDE maybe have a hxml & xml HX_THREAD_API_XML define which you can point to a custom xml file which will provide the implementation of the ConditionVariable and RecursiveMutex types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable, though if it is for hxml use it should be prefixed with HXCPP_ for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's only one use of HxMutex / a bit of manual pthread condition left in Immix.cpp now, unfortunately this last one is a bit of a pain. C++ states that when the dtor of std::condition_variable gets called no thread should have it locked, however on exit hxcpp programs hang when adapting the last uses of pthread conditions to std::condition_variable as some thread seems to be holding onto the locks.
Replaces the old implementation which had a bunch of ifdefs depending on the platform. Now we have a single implementation using the C++
stdtypes. I would have done the same forLockbut for some reason it took until C++20 to get a semaphore type...I have broken the definitions out into their own headers and exposed the classes so on the haxe side we can remove the untyped code.