Skip to content

Replaced POSIX threads under Windows with stdlib implementations since it is under LGPL license#98

Closed
holoeye-photonics wants to merge 0 commit into
oxygine:masterfrom
holoeye-photonics:master
Closed

Replaced POSIX threads under Windows with stdlib implementations since it is under LGPL license#98
holoeye-photonics wants to merge 0 commit into
oxygine:masterfrom
holoeye-photonics:master

Conversation

@holoeye-photonics

Copy link
Copy Markdown
Contributor

The oxygine engine uses POSIX threads under Windows which is a handy library but sadly under LGPL so we cannot link it statically. Since C++ 11 there are official implementations std::mutex, std::thread and std::conditional_variable. So I put those in which gets rid of POSIX threads under Windows which resolves our licensing issue. There is a define OX_CPP11THREADS that needs to be set in order to enable the use of the stdlib classes.

@holoeye-photonics

Copy link
Copy Markdown
Contributor Author

I had to split the mutex class into Mutex and MutexRecursive since the recursive mutex in stdlib is its own type.
I tried a templated autolock class but it did not work nicely so I ended up having two autolock classes, one for Mutex and one for MutexRecursive.

@frankinshtein

frankinshtein commented Apr 26, 2017

Copy link
Copy Markdown
Contributor

wow! good job
Most complicated part here is ThreadDispatcher.
Have you tested it?

@holoeye-photonics

Copy link
Copy Markdown
Contributor Author

I noticed that. The ThreadDispatcher is probably still a bit fishy.

When you look at the file you see in line 156, that I put in a todo.

The problem is that POSIX allows you to have a condition for a recursive mutex. The stdlib does not support this. There is a type conditional_variable_any that would take a recursive mutex, but the documentation explicitly says that this will not work.

So I would see two solutions here. Either use two mutex, one for locking one only to wait for the condition, or we use like an atomic int to establish a communication.

My current workaround looks like this which isprobably not safe.
MutexPthreadLock::MutexPthreadLock(std::unique_lock<std::mutex>& l, bool lock) : _lock(l), _locked(false) 27 { 28 if(lock && !_lock.owns_lock()) 29 { 30 _lock.lock(); 31 _locked = true; 32 } 33 }

Do you have a unit test or something that could be used to very if no problem exists? Even just some instructions on how to test this would be fine.
As I said we need the stdlib threads due to licensing issues so I am happy to look at this more and to actually resolve any remaining issues with ThreadDispatcher.

Cheers,
Dan

@holoeye-photonics

Copy link
Copy Markdown
Contributor Author

I created a new pull request to keep the changes clean.

#100

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 this pull request may close these issues.

2 participants