configure: Fix thread_local detection #16059

pull dongcarl wants to merge 1 commits into bitcoin:master from dongcarl:2019-05-fix-thread_local-compat changing 1 files +49 −36
  1. dongcarl commented at 7:19 PM on May 20, 2019: member
    • When aiming for glibc compatibility, don't use thread_local. Fixes #15958.
    • FreeBSD has a buggy thread_local, don't use it. Fixes #16055.

    I've done a Gitian build on my local machine and the symbol tests seem to pass.

  2. dongcarl added the label Bug on May 20, 2019
  3. dongcarl added the label Build system on May 20, 2019
  4. dongcarl added the label Needs gitian build on May 20, 2019
  5. MarcoFalke added this to the milestone 0.19.0 on May 20, 2019
  6. MarcoFalke commented at 7:34 PM on May 20, 2019: member

    ACK 3b549f4aae9bc5f5f9e9dee8476523daf44dd7ee (checked diff with --ignore-all-space, and verified that thread_local is now disabled on freebsd)

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 3b549f4aae9bc5f5f9e9dee8476523daf44dd7ee (checked diff with --ignore-all-space, and verified that thread_local is now disabled on freebsd)
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUhhlwwAp1D8oI5zuBCaduML4SFgcShnTo6EeWUN6VrVQDj0Ja4d2iEvgog3iJP1
+KMwj0frmEKmoBxOSW3b72iTPRWT/TeYXgp/PgqAIvy8E+K6CCIdDyBY+c/2iZTc
PSN/GnDkHGNhJ8Cx+RCgB4Io6/df+44XgpvLNzBrFeLkuZhSE0RuzzwIG9NJ3lpY
AEI/OAsiAZLZP23scVqo7z3T9rYM0uUtW5XpW98eJSYlQ+JFe0DE7/oKyeETqVRE
GZ6XtAf+rVEZ/8awa7QBcfFmQ2qxYNQGPewudTVRR9//PhFBn2jCvmqDvg26+zA6
CAkDDcfQb4NLIjvnviZxkQhaXUPdc+PaefCD6Pzx1qL0bUsY2zIUYnw1fV8uZ69K
YTZBewMyJRDQE8oz0NB+9xSpftG8yUlY2H4Xrwo9IEBfGwGsITAc1dv61lfrGrDO
hGC3bE8ucDb+IaClCtrBS3sOyxqRfS3BjrgjkmA4QUJYyXLsyvNYZeTWpcfhKopm
83ydESNX
=eGHB
-----END PGP SIGNATURE-----

    Timestamp of file with hash cb3987fa66dbad9cd23f72f8b0cb42d860ccda592d0a20757c01c728c5eadf2e -

    </details>

  7. jamesob approved
  8. jamesob commented at 8:01 PM on May 20, 2019: member

    utACK https://github.com/bitcoin/bitcoin/pull/16059/commits/3b549f4aae9bc5f5f9e9dee8476523daf44dd7ee

    To confirm: use_glibc_compat is by default disabled, correct?

  9. dongcarl commented at 8:03 PM on May 20, 2019: member

    @jamesob

    To confirm: use_glibc_compat is by default disabled, correct?

    Correct! https://github.com/bitcoin/bitcoin/blob/2d1583ee6aff4b68dc46bcb87eb7a85f90b465c3/configure.ac#L191-L195

  10. theuni commented at 8:04 PM on May 20, 2019: member

    Concept ACK, but I think that the logic here is complex enough that an --enable-threadlocal is necessary so that it can be chosen explicitly.

  11. dongcarl commented at 8:09 PM on May 20, 2019: member

    @theuni

    Concept ACK, but I think that the logic here is complex enough that an --enable-threadlocal is necessary so that it can be chosen explicitly.

    As in:

    1. Keep my current changes, add a --enable-threadlocal flag that will enable thread_local regardless of what --enable-glibc-back-compat says?
    2. Or... Keep my current changes, add a --enable-threadlocal flag that will fail the configure if --enable-glibc-back-compat is also there?
    3. Or... Remove the use_glibc_compat <-> thread_local interaction, add a --enable-threadlocal flag that will enable thread_local?
  12. MarcoFalke commented at 8:16 PM on May 20, 2019: member

    I think it is nice to have thread_local enabled by default on the generic "64-bit Ubuntu Bionic" (and similar) environments. Thread names give developers additional debug information.

  13. ryanofsky commented at 8:18 PM on May 20, 2019: member

    Option (1) is the way to go. If a user passes explicitly passes --enable-threadlocal or --disable-threadlocal, it should enable or disable thread local code respectively, regardless of other options. If neither is specified, it should choose an intelligent default based on the platform and maybe based on --glibc-back-compat.

  14. theuni commented at 8:33 PM on May 20, 2019: member

    Option (1) is the way to go. If a user passes explicitly passes --enable-threadlocal or --disable-threadlocal, it should enable or disable thread local code respectively, regardless of other options. If neither is specified, it should choose an intelligent default based on the platform and maybe based on --glibc-back-compat.

    +1

    Thread names give developers additional debug information.

    They had thread names before thread_local :)

  15. jamesob commented at 9:11 PM on May 20, 2019: member

    They had thread names before thread_local :)

    I think @MarcoFalke is referring to the [threadname] prefixes on each log line, which did not exist before the merge of #15849.

  16. dongcarl force-pushed on May 20, 2019
  17. dongcarl commented at 9:29 PM on May 20, 2019: member

    Implemented logic as documented here: #16059 (comment)

  18. in configure.ac:199 in caaa942a5f outdated 
     193 | @@ -194,6 +194,12 @@ AC_ARG_ENABLE([glibc-back-compat],
 194 |    [use_glibc_compat=$enableval],
 195 |    [use_glibc_compat=no])
 196 |  
 197 | +AC_ARG_ENABLE([threadlocal],
 198 | +  [AS_HELP_STRING([--enable-threadlocal],
 199 | +  [enable thread_local code])],
    ryanofsky commented at 9:42 PM on May 20, 2019:

    Maybe could have more descriptive help text "enable features that depend on the c++ thread_local keyword (currently just thread names in debug logs). If unspecified, this will be enabled if there is platform support and glibc-back-compat is not enabled"

  19. DrahtBot commented at 9:13 PM on May 21, 2019: member

    <!--a722867cd34abeea1fadc8d60700f111-->

    Gitian builds for commit 2d1583ee6aff4b68dc46bcb87eb7a85f90b465c3 (master):

    Gitian builds for commit 256e35348968dc048f175507999c466654294194 (master and this pull):

  20. DrahtBot removed the label Needs gitian build on May 21, 2019
  21. dongcarl added the label Needs gitian build on May 21, 2019
  22. DrahtBot commented at 1:28 PM on May 22, 2019: member

    <!--a722867cd34abeea1fadc8d60700f111-->

    Gitian builds for commit 3001cc61cf11e016c403ce83c9cbcfd3efcbcfd9 (master):

    Gitian builds for commit 3f02161dd63d4983ae2b356dfea3dd4e45ee1f4a (master and this pull):

  23. DrahtBot removed the label Needs gitian build on May 22, 2019
  24. MarcoFalke commented at 1:57 PM on May 22, 2019: member

    utACK caaa942a5ff372db723a224c35236df48b93ebcb

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

utACK caaa942a5ff372db723a224c35236df48b93ebcb
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUgOcAwAqOf1q52e0OmUrUy+lg0FHlFZ/F4VXVN/NtRAa7wP1KO+GsQacvFuKlOT
7T9V7KFqdRbj9RsxwXd77ZwXkhA7PDOn5Z8ja4GvnH/Z8KXOOKyPKJF9Bw6qtUH0
7ghGVa1eawGNddId5WQfhdJygwjbuwPZbqkxY3dLW1puXXXV7+RkWnEz6c+eZG2b
zJ84SlRd8BvGDcRqSoSGtH4kwXbfjYwQMk1+wt1j534/UgHq4ubHOPiSq38GMF2X
PmK6LGlG0PO9sU4hV5a62k5aZ0cW22n3HlL6MVTODAkSci7Cfd2Mo84dNC8TevOU
NU+J5wTaqDn84tLKRBWvPdtH3pb1L2WtKVPQ1sVCFrXFsguGnPhKy8r7rB8dgjkz
efLYbXO8vGK1GRTqsFsyAsxi7/AMS4PyIG19nnazGw2WmVIqcRp7D6ZTE+uEpCyV
Z7cB4UgtZo9MdrwJqmjkbHXD45NHjxvy5SsXtVRcWzQ2J50BsCn+rfLZxkc484jJ
srQR/1SK
=9lnG
-----END PGP SIGNATURE-----

    Timestamp of file with hash 92094b899f3571c01b98c4d14e9586500a17402059ac90ca5200d93dc689eef3 -

    </details>

  25. dongcarl commented at 3:38 PM on May 22, 2019: member

    From the @DrahtBot gitian builds, looks like bitcoin-core-linux-0.19-res.ymls are coming back again! :tada:

  26. sipa commented at 6:32 PM on May 22, 2019: member

    Concept ACK

  27. ryanofsky approved
  28. ryanofsky commented at 5:20 PM on May 23, 2019: member

    utACK caaa942a5ff372db723a224c35236df48b93ebcb. Change looks good to me. I just suggested adding some more documentation below. IMO, the glibc-back-compat option is also underdocumented, and is perhaps trying to control too many disparate things. My instinct would be to make the --enable-threadlocal and --enable-glibc-back-compat options independent and just pass them both where needed. But I don't know all the context, and the change seems fine in its present form.

  29. laanwj added this to the "Blockers" column in a project
  30. configure: Add flag for enabling thread_local.
    - When aiming for glibc compatibility, don't use thread_local.
- Add a flag --enable-threadlocal, which, when specified, will
  enable/disable thread_local regardless of the value of glibc_compat.
- FreeBSD has a buggy thread_local, don't use it.
    480e3415d7
  31. dongcarl force-pushed on May 23, 2019
  32. dongcarl commented at 7:16 PM on May 23, 2019: member

    Update: better help text.

  33. MarcoFalke commented at 11:24 AM on May 24, 2019: member

    utACK 480e3415d738a4e0e2ad9774c43f29937178ecae

  34. fanquake commented at 5:07 PM on May 24, 2019: member

    tACK 480e341

    macOS:

    checking for thread_local support... no
src/bitcoind -logthreadnames=1
2019-05-23T20:27:48Z [] Bitcoin Core version v0.18.99.0-480e3415d (release build)
2019-05-23T20:27:48Z [] Assuming ancestors of block 0000000000000000000f1c54590ee18d15ec70e68c8cd4cfbadb1b4f11697eee have valid signatures.

    openBSD 6.4:

    checking for thread_local support... yes
./src/bitcoind -logthreadnames=1
2019-05-23T20:41:11Z [init] mapBlockIndex.size() = 1
2019-05-23T20:41:11Z [loadblk] Failed to open mempool file from disk. Continuing anyway.
......
2019-05-23T20:41:58Z [shutoff] [default wallet] Releasing wallet
2019-05-23T20:41:58Z [shutoff] Shutdown: done

    FreeBSD 12.0:

    checking for thread_local support... no
src/bitcoind -logthreadnames=1
2019-05-24T16:06:38Z [] Bitcoin Core version v0.18.99.0-480e3415d (release build)
2019-05-24T16:06:38Z [] Assuming ancestors of block 0000000000000000000f1c54590ee18d15ec70e68c8cd4cfbadb1b4f11697eee have valid signatures.

    Debian 9.9:

    checking for thread_local support... yes
src/bitcoind -logthreadnames=1
src/bitcoind -logthreadnames=1
2019-05-24T17:07:14Z [init] Bitcoin Core version v0.18.99.0-480e3415d (release build)
2019-05-24T17:07:14Z [init] Assuming ancestors of block 0000000000000000000f1c54590ee18d15ec70e68c8cd4cfbadb1b4f11697eee have valid signatures.
  35. MarcoFalke merged this on May 25, 2019
  36. MarcoFalke closed this on May 25, 2019
  37. MarcoFalke referenced this in commit e043bfce68 on May 25, 2019
  38. fanquake removed this from the "Blockers" column in a project
  39. laanwj referenced this in commit acb4fa0741 on Apr 22, 2020
  40. sidhujag referenced this in commit 36106e9ef0 on Apr 23, 2020
  41. linuxsh2 referenced this in commit 3c6ae97332 on Aug 11, 2021
  42. DrahtBot locked this on Dec 16, 2021
Contributors
dongcarlMarcoFalkejamesobtheuniryanofskyDrahtBotsipafanquake
Labels
BugBuild system

Milestone
0.19.0

Linked (view graph)
#15958 32 bit linux gitian build fails with "symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18"#16055 bitcoind on freebsd prints "__cxa_thread_call_dtors: dtr 0x801a85a40 from unloaded dso, skipping"#18665 Do not expose and consider -logthreadnames when it does not work
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 21:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me