Remove calls to getaddrinfo_a #9229
Conversation
|
This should probably be backported. |
|
@theuni can you take a look here? |
| @@ -512,7 +512,6 @@ if test x$TARGET_OS = xdarwin; then | |||
| fi | |||
|
|
|||
| AC_CHECK_HEADERS([endian.h sys/endian.h byteswap.h stdio.h stdlib.h unistd.h strings.h sys/types.h sys/stat.h sys/select.h sys/prctl.h]) | |||
| AC_SEARCH_LIBS([getaddrinfo_a], [anl], [AC_DEFINE(HAVE_GETADDRINFO_A, 1, [Define this symbol if you have getaddrinfo_a])]) | |||
| AC_SEARCH_LIBS([inet_pton], [nsl resolv], [AC_DEFINE(HAVE_INET_PTON, 1, [Define this symbol if you have inet_pton])]) | |||
There was a problem hiding this comment.
Interesting. It looks like this detection goes wrong on windows, and we only notice now because all uses were inside HAVE_GETADDRINFO_A.
On windows you apparently have to use InetPton: https://msdn.microsoft.com/en-us/library/windows/desktop/cc805844(v=vs.85).aspx
But this is only available on Vista and higher so the safest just to not use it on windows.
|
On windows 32 bit (works correctly there):
On windows 64 bit:
What is this "none required"? It detects the symbol in the C library? Edit: I checked and apparently the symbol is in |
|
Looks like pretty much all the code being removed here was added in #4421 re 'none required'. See here - https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Libraries.html |
|
No real preference here. I'm just a few PRs away from removing the current resolve/connection code in favor of async lookups/connections. Given that our current connection model is very synchronous as-is, I've been curious for a while as to whether getaddrinfo_a is worth the trouble. So, concept ACK for nuke/backport. |
This reverts commit caf6150. getaddrinfo_a has a nasty tendency to segfault internally in its background thread, on every version of glibc I tested, especially under helgrind. See https://sourceware.org/bugzilla/show_bug.cgi?id=20874
e52c4cb to
10ae7a7
Compare
|
Switched to a wholesale revert of #4421 as discussed in meeting. |
|
I've managed to reproduce the issue and posted to the sourceware thread. |
10ae7a7 Revert "Use async name resolving to improve net thread responsiveness" (Matt Corallo)
This reverts commit caf6150. getaddrinfo_a has a nasty tendency to segfault internally in its background thread, on every version of glibc I tested, especially under helgrind. See https://sourceware.org/bugzilla/show_bug.cgi?id=20874 Github-Pull: #9229 Rebased-From: 10ae7a7
|
pushed to 0.13 branch as b172377, removing backport tag |
10ae7a7 Revert "Use async name resolving to improve net thread responsiveness" (Matt Corallo)
This reverts commit caf6150. getaddrinfo_a has a nasty tendency to segfault internally in its background thread, on every version of glibc I tested, especially under helgrind. See https://sourceware.org/bugzilla/show_bug.cgi?id=20874 Github-Pull: bitcoin#9229 Rebased-From: 10ae7a7
10ae7a7 Revert "Use async name resolving to improve net thread responsiveness" (Matt Corallo)
10ae7a7 Revert "Use async name resolving to improve net thread responsiveness" (Matt Corallo)
d46b369 [net] Avoid initialization to a value that is never read Prior to this commit the value stored to `s` at initialization was never read (in the case of STRERROR_R_CHAR_P). (furszy) f62d152 Revert "Use async name resolving to improve net thread responsiveness" (furszy) a1b0017 Use Socks5ErrorString() to decode error responses from socks proxy. (furszy) fdec2e3 Make Socks5() InterruptibleRecv() timeout/failures informative. Before: ERROR: Error reading proxy response (furszy) 2027d66 SOCKS5 connecting and connected messages with -debug=net. They were too noisy and not necessary for normal operation. (furszy) 097322c Make failures to connect via Socks5() more informative and less unnecessarily scary. * The "ERROR" was printed far too often during normal operation for what was not an error. (furszy) Pull request description: Made a set of back ports to make Socks5 connection failures less noisy and less unnecessarily scary. Plus added a straightforward removal of `getaddrinfo_a` which was never available at configure time. Back ported PRs list: bitcoin#8033 bitcoin#9229 bitcoin#9539 ACKs for top commit: Fuzzbawls: utACK d46b369 random-zebra: utACK d46b369 and merging... Tree-SHA512: 5b79309a2bc9ffca686e00b431d8205c72c353a56693b7f1e7a0a7245b158032e9243a49fea10a606cb2329726b3908fd5187ac6c8e57ddd0e9025a9811950ec
getaddrinfo_a has a nasty tendency to segfault internally in its
background thread, on every version of glibc I tested, especially
under helgrind.
See https://sourceware.org/bugzilla/show_bug.cgi?id=20874