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

cmake: usrsctplib: link against threads library #644

Merged
merged 1 commit into from
Jan 7, 2022

Conversation

klemensn
Copy link
Contributor

@klemensn klemensn commented Jan 2, 2022

The usrsctp library uses threads, but building it with cmake does not
link against the system's thread library.

Use cmake's "Threads" package as usual to deal with all supported
platforms.
(autoconf's configure.ac correctly adds -pthread to CFLAGS.)

This is needed because programs which do not use threads themselves but
link against usrsctplib will otherwise fail to link since pthread_*
symbols used in libusrsctp.so are not resolved.

programs/CMakeFiles.txt avoids this problem by unconditionally using the
"Threads" package for every program including those that do not use
threads themselves.

@tuexen
Copy link
Member

tuexen commented Jan 2, 2022

@weinrank : What do you think?

@weinrank
Copy link
Contributor

weinrank commented Jan 7, 2022

The change successfully adds the "-pthread" flag as a compiler option, I'm fine with it.

Do we also need this flag for Apple?

General question, since I've not clue about the pthread flag itself: why do we need it?
Do you face any compiler/linker errors? I'm not arguing against the flag and after googling/reading man pages, I think it's a good idea to add it. I'm just questioning myself why we haven't seen any errors/issues for the past years of omitting the flag.

Best

The usrsctp library uses threads, but building it with cmake does not
link against the system's thread library.

Use cmake's "Threads" package as usual to deal with all supported
platforms.
(autoconf's configure.ac correctly adds `-pthread` to CFLAGS.)

This is needed because programs which do not use threads themselves but
link against usrsctplib will otherwise fail to link since `pthread_*`
symbols used in libusrsctp.so are not resolved.

programs/CMakeFiles.txt avoids this problem by unconditionally using the
"Threads" package for every program including those that do not use
threads themselves.
@klemensn
Copy link
Contributor Author

klemensn commented Jan 7, 2022

The change successfully adds the "-pthread" flag as a compiler option, I'm fine with it.

Do we also need this flag for Apple?

I don't know, but my udpated diff lets cmake's "Threads" package handle all platform specific bits, so that is no longer our problem.

General question, since I've not clue about the pthread flag itself: why do we need it? Do you face any compiler/linker errors? I'm not arguing against the flag and after googling/reading man pages, I think it's a good idea to add it. I'm just questioning myself why we haven't seen any errors/issues for the past years of omitting the flag.

The new commit message hopefully explains it better.
tl;dr: Unless your program linking against libusrsctp.so (which does not link against phtreads itself) uses -pthreads or -lpthreads, it will either fail to link at compile-time with something like

ld: error: ./usrsctplib/libusrsctp.so: undefined reference to pthread_rwlock_rdlock [--no-allow-shlib-undefined]
[ more undefined references to other pthread_* symbols ]

or it'll fail to run eventually:

test:/usr/local/lib/libusrsctp.so.0.1: undefined symbol 'pthread_mutexattr_init'                         
ld.so: test_libmgmt: lazy binding failed!                                                                        
Killed

@klemensn klemensn changed the title cmake: Build with -pthread on Linux and BSD cmake: usrsctplib: link against threads library Jan 7, 2022
@tuexen
Copy link
Member

tuexen commented Jan 7, 2022

OK, so the point is that you want to deal with -pthread at the library level, not at the application level as we do today.

Is that is correct?

@tuexen tuexen merged commit 40e35a7 into sctplab:master Jan 7, 2022
klemensn added a commit to klemensn/usrsctp that referenced this pull request Jan 7, 2022
Follow 40e35a7 "cmake: usrsctplib: link against threads library (sctplab#644)";
this is not in "Libs:" since programs do not need to link against
pthreads themselves.

See https://people.freedesktop.org/~dbn/pkg-config-guide.html
tuexen pushed a commit that referenced this pull request Jan 7, 2022
Follow 40e35a7 "cmake: usrsctplib: link against threads library (#644)";
this is not in "Libs:" since programs do not need to link against
pthreads themselves.

See https://people.freedesktop.org/~dbn/pkg-config-guide.html
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.

None yet

3 participants