dougbinks / enkiTS

A permissively licensed C and C++ Task Scheduler for creating parallel programs. Requires C++11 support.
zlib License
1.74k stars 145 forks source link

Valgrind errors on OSX High Sierra #49

Closed boxerab closed 4 years ago

boxerab commented 4 years ago

Hi Doug, I ran my image codec, grok, which uses enkiTS, on OSX with valgrind, and I see this error:

==3380== Process terminating with default action of signal 11 (SIGSEGV)
==3380==  Access not within mapped region at address 0x18
==3380==    at 0x100CC05BA: _pthread_body (in /usr/lib/system/libsystem_pthread.dylib)
==3380==    by 0x100CC050C: _pthread_start (in /usr/lib/system/libsystem_pthread.dylib)
==3380==    by 0x100CBFBF8: thread_start (in /usr/lib/system/libsystem_pthread.dylib)
==3380==  If you believe this happened as a result of a stack
==3380==  overflow in your program's main thread (unlikely but
==3380==  possible), you can try to increase the size of the
==3380==  main thread stack using the --main-stacksize= flag.
==3380==  The main thread stack size used in this run was 8388608.

This is for an earlier version of enkiTS, as the latest version will result in BAD_ACCESS error and my program crashes.

Have you run any valgrind tests on OSX ? Everything looks good on Linux.

Thanks.

dougbinks commented 4 years ago

Are you able to run the examples with enkiTS, and do you get the same results from valgrind? If not, this may be an usage issue so if I could take a look at the codec code I might be able to help.

dougbinks commented 4 years ago

I can replicate a segfault 11 on OSX 10.11.6 (El Capitan), which is the latest I can access for OSX as I don't have a new enough machine for 10.13 (High Sierra). I'm looking into this now but it might take a few days, particularly as my OSX machine is not very stable at the moment.

dougbinks commented 4 years ago

So it looks like OSX have deprecated mach semaphores, as they worked under previous versions but now I've updated OSX to a more recent version they don't.

I'll try switching to OSX dispatch semaphores, if this doesn't work I may need to switch to named BSD semaphores and use a unique name approach.

boxerab commented 4 years ago

Thanks! C++20 finally provides a semaphore, BTW 😊

boxerab commented 4 years ago

Side note: this would have been caught by CI builds. If you're interested, I can look into setting up Travis and Appveyor CI for the project

dougbinks commented 4 years ago

I'm already using Travis Cl for OSX, and this wasn't caught by my Cl builds since it's a runtime error rather than a build one. I need to create a simple test which can be run on the build server, and add that to the matrix.

boxerab commented 4 years ago

Great to have CI, yes, I guess next step would be a few unit tests.

boxerab commented 4 years ago

btw, for my testing, I installed OSX High Sierra VM on Ubuntu host using qemu:

https://github.com/kholia/OSX-KVM

Took about an hour to get this working.

dougbinks commented 4 years ago

I might be able to do this by running a VM on my Mac, as I believe this is permitted by the OSX license. However my Mac's not very powerful nor very stable so this may not be a great experience, but it could suffice.

dougbinks commented 4 years ago

The new implementation uses dispatch semaphores, which appears to be the approach used by other open source utilities such as boost.

This requires OSX 10.6+ or iOS 4+ so if any developers need support prior to that I might reinstate mach semaphores but would need someone to test as I don't have earlier versions to check the macro tests work.

At some point I'll add a simple unit test which should catch OSX changes to API behaviour.

boxerab commented 4 years ago

Thanks!! Works like a charm.