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

Extend PR #169 #220

Open
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@arogge
Copy link
Contributor

commented Jul 11, 2019

This is essentially PR #169 with changes to compile on FreeBSD and other systems.

@arogge arogge force-pushed the dev/arogge/master/fix-1073 branch from e7ca5a1 to 76f16d9 Jul 11, 2019

bratkartoffel and others added some commits Apr 18, 2019

dird: Fix pthread issues
Fixes two double pthread_detach() calls and one pthread_getspecific()
without initialization, all leading to a SEGFAULT with musl libc and
undefined behaviour on other libc implementations.

This closes Bug #1073
core: Fixes to PR#169
- move DetachIfNotDetached() to thread.cc
- detect pthread_attr_get_np() vs. pthread_getattr_np()
- add ifdefs for FreeBSD
- just detach without check on Win32

@arogge arogge force-pushed the dev/arogge/master/fix-1073 branch from 76f16d9 to 650e8dd Jul 12, 2019

@arogge arogge marked this pull request as ready for review Jul 15, 2019

@arogge arogge requested a review from franku Jul 15, 2019

@franku
Copy link
Contributor

left a comment

Minor requests as these are cmake and filename suggestions.

Show resolved Hide resolved core/src/dird/thread.cc Outdated
Show resolved Hide resolved core/src/dird/thread.cc Outdated
Show resolved Hide resolved core/cmake/thread.cmake
Show resolved Hide resolved core/cmake/thread.cmake
Show resolved Hide resolved core/cmake/thread.cmake Outdated
Show resolved Hide resolved core/src/dird/CMakeLists.txt Outdated
Show resolved Hide resolved core/src/dird/thread.cc Outdated
@@ -87,3 +87,4 @@ if (${ZLIB_FOUND})
endif()

find_package(Readline)
INCLUDE(thread)

This comment has been minimized.

Copy link
@franku

franku Jul 15, 2019

Contributor
Suggested change
INCLUDE(thread)
INCLUDE(pthread_detach_if_not_detached)

This comment has been minimized.

Copy link
@arogge

arogge Jul 15, 2019

Author Contributor

These cmake variables are not specific for this functionality, but generic variables for pthread usage, so it should be "thread" or "pthread"

Show resolved Hide resolved core/src/dird/dird.h Outdated
core: implement changes from PR review
- rename file
- use cmake push/pop
- remove bogus prototype

@arogge arogge force-pushed the dev/arogge/master/fix-1073 branch from c4617c6 to b8fb57a Jul 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.