Overlinking: Currently libreplace links against librt, but it isn't needed as the symbols we are looking for are provided by glibc. The replace waf script checks for pthread and sets LDFLAGS=-pthread globally. However most of our code does not need this at all. Impact on NSS modules: NSS modules are more fragile and during and update they can be non-functional for some time. Like with nss-winbind it wont work till all libraries have been upgraded and winbind installed and restarted. During an upgrade the libnss_winbind and libnss_wins, known as the "winbind" and "wins" service providers for NSS, can trigger a glibc defect which causes the upgrade to fail. The reason is that librt and libpthread are marked as NODELETE so they wont be reloaded when a new nss_winbind has been installed. We can avoid the glibc bug if we link those modules only against strictly necessary libraries. nss_winbind and nss_wins don't have to be linked against librt nor libpthread at all. RH Samba bug: https://bugzilla.redhat.com/show_bug.cgi?id=1754575 RH glibc bug: https://bugzilla.redhat.com/show_bug.cgi?id=1748197
This was (probably) changed in 4.11 by ea7231dcc0b50c535f913f0542d600d0b2119a21. "libwbclient: Use wrapper for string to integer conversion". Side note: I remember Ubuntu thinking about making libwbclient statically linked (but couldn't find the link).
Note that we dropped the nss_wins part as we couldn't avoid pthread there since it depends on samba-util library which needs it (it also depends on krb5 libs which are linked against pthread).
Created attachment 15492 [details] patch for 4.11
Created attachment 15493 [details] patch for 4.10
Comment on attachment 15492 [details] patch for 4.11 LGTM
Comment on attachment 15493 [details] patch for 4.10 LGTM
Karolin, could you please the patches to the corresponding branches? Thanks!
(In reply to Andreas Schneider from comment #7) Pushed to autobuild-v4-{11,10}-test.
(In reply to Karolin Seeger from comment #8) The 4.10 build failed with: ../../nsswitch/stress-nss-libwbclient.c:135: error: undefined reference to 'pthread_create' ../../nsswitch/stress-nss-libwbclient.c:142: error: undefined reference to 'pthread_create' ../../nsswitch/stress-nss-libwbclient.c:152: error: undefined reference to 'pthread_join' collect2: error: ld returned 1 exit status Build failed -> task in 'stress-nss-libwbclient' failed with exit status 1 (run with -v to display more information) make: *** [all] Error 1
Created attachment 15542 [details] wip patch for stress-nss-libwbclient (In reply to Stefan Metzmacher from comment #9) I think stress-nss-libwbclient should link against pthread, the reason we don't see it on master is because on master its wbclient dependency links against samba-util (libwbclient/wscript) so pthread gets pulled in anyway, but on that branch it hand't happen yet so it fails to build. Attached patched works for me, pipeline: https://gitlab.com/samba-team/devel/samba/tree/iboukris-pthread-stress-nss-libwbclient
Pipeline for 4.10 build: https://gitlab.com/samba-team/devel/samba/pipelines/88930283
Comment on attachment 15542 [details] wip patch for stress-nss-libwbclient This looks change looks good. Andreas, can you replace 'wip:' with 'nsswitch:', add the bug reference and our review and push this to master? I think we should revert ea7231dcc0b50c535f913f0542d600d0b2119a21 (libwbclient: Use wrapper for string to integer conversion) from master and 4.11. libwbclient is supposed to have minimal dependencies. The problem is bug tries to fix gets worse with ea7231dcc0b50c535f913f0542d600d0b2119a21.
(In reply to Stefan Metzmacher from comment #12) @metze take a look at: https://gitlab.com/samba-team/samba/merge_requests/847 +1 for minimal deps in nss modules, we gave up nss_wins as it links to krb5 libs and pulls pthread from there anyway.
Created attachment 15551 [details] additional patch for 4.11
Created attachment 15552 [details] new patch for 4.10
(In reply to Andreas Schneider from comment #14) Pushed to autobuild-v4-11-test.
(In reply to Andreas Schneider from comment #15) Does not apply on current v4-10-test.
(In reply to Karolin Seeger from comment #17) Pushed to v4-10-test.
Pushed to both branches. Closing out bug report. Thanks!
Created attachment 15561 [details] additional patch for 4.10 to remove shmget and shm_open checks
Comment on attachment 15561 [details] additional patch for 4.10 to remove shmget and shm_open checks LGTM
Karolin, could you please apply the new patch to v4.10? Thanks!
(In reply to Andreas Schneider from comment #22) Pushed additional test to autobuild-v4-10-test.
(In reply to Karolin Seeger from comment #23) Pushed to v4-10-test. Closing out bug report. Thanks!