investigate if getaddrinfo(3) on OSX is thread-safe

[ronaldoussoren]

BPO	25924
Nosy	@gvanrossum, @ronaldoussoren, @ned-deily, @vadmium, @1st1, @ajdavis
Files	h_resolv.c: getaddrinfo concurrency test from NetBSD 7 d_mach: getaddrinfo concurrency test data file from NetBSD 7 25924-getaddrinfo-is-thread-safe.patch: Disable getaddrinfo lock on Mac OS 10.5+. 25924-getaddrinfo-is-thread-safe-2.patch: Disable getaddrinfo lock on Mac OS 10.5+, try 2. 25924-getaddrinfo-is-thread-safe-3.patch: Disable getaddrinfo lock on Mac OS 10.5+, try 3 (runtime check).

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ronaldoussoren'
closed_at = <Date 2016-02-15.06:37:31.629>
created_at = <Date 2015-12-22.12:57:10.941>
labels = ['OS-mac', 'type-feature']
title = 'investigate if getaddrinfo(3) on OSX is thread-safe'
updated_at = <Date 2016-11-17.12:27:39.268>
user = 'https://github.com/ronaldoussoren'

bugs.python.org fields:

activity = <Date 2016-11-17.12:27:39.268>
actor = 'emptysquare'
assignee = 'ronaldoussoren'
closed = True
closed_date = <Date 2016-02-15.06:37:31.629>
closer = 'ned.deily'
components = ['macOS']
creation = <Date 2015-12-22.12:57:10.941>
creator = 'ronaldoussoren'
dependencies = []
files = ['41596', '41597', '41755', '41784', '41878']
hgrepos = []
issue_num = 25924
keywords = ['patch']
message_count = 21.0
messages = ['256838', '257610', '258121', '258125', '258163', '259233', '259234', '259387', '259404', '259424', '259437', '259457', '259480', '259967', '259970', '259974', '260242', '260297', '260299', '260731', '281036']
nosy_count = 7.0
nosy_names = ['gvanrossum', 'ronaldoussoren', 'ned.deily', 'python-dev', 'martin.panter', 'yselivanov', 'emptysquare']
pr_nums = []
priority = 'low'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue25924'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

[ronaldoussoren]

https://emptysqua.re/blog/getaddrinfo-deadlock/ claims that getaddrinfo may deadlock when using threads and fork on (amongst others) OSX due to using a global lock.

That lock is used when getaddrinfo is believed to be not thread safe, see the relevant code below (from the blog post):

/* On systems on which getaddrinfo() is believed to not be thread-safe,
   (this includes the getaddrinfo emulation) protect access with a lock. */
#if defined(WITH_THREAD) && (defined(__APPLE__) || \
    (defined(__FreeBSD__) && __FreeBSD_version+0 < 503000) || \
    defined(__OpenBSD__) || defined(__NetBSD__) || \
    defined(__VMS) || !defined(HAVE_GETADDRINFO))
#define USE_GETADDRINFO_LOCK
#endif

I think it is worthwhile to investigate whether or not getaddrinfo on OSX is really not thread safe.

Some source code for OSX can be found at the link below, I haven't checked yet which OSX release this corresponds to:

http://www.opensource.apple.com/source/Libinfo/Libinfo-278/lookup.subproj/getaddrinfo.c

[ajdavis]

In Apple's Libinfo version 222.4.12 (corresponding to the last OS X 10.4 release), the man page says getaddrinfo isn't thread-safe:

http://www.opensource.apple.com/source/Libinfo/Libinfo-222.4.12/lookup.subproj/getaddrinfo.3

And here's its source:

http://www.opensource.apple.com/source/Libinfo/Libinfo-222.4.12/lookup.subproj/getaddrinfo.c

Glancing at the source naïvely, I might see the data race: getaddrinfo calls gai_lookupd, which reads and writes the global static variable "gai_proc". I can't see what will go wrong as a result of the race, but it sure LOOKS bad.

In the next release, version 278 (OS X 10.5.0), the thread-safety warning is gone from the man page:

http://www.opensource.apple.com/source/Libinfo/Libinfo-278/lookup.subproj/getaddrinfo.3

And getaddrinfo is largely rewritten:

http://www.opensource.apple.com/source/Libinfo/Libinfo-278/lookup.subproj/getaddrinfo.c

It calls a new function, "ds_getaddrinfo". But ds_getaddrinfo still accesses the global static variable "gai_proc"; I wonder why this is considered thread-safe now?

[ajdavis]

Related to bpo-1288833, when FreeBSD 5.3's getaddrinfo was declared thread-safe.

[ajdavis]

NetBSD has a concurrency test for getaddrinfo, so I tried it on Mac to see if getaddrinfo is thread-safe here, too. It appears that it is thread-safe.

I copied NetBSD's getaddrinfo concurrency test "h_resolv.c" and the test's data file "d_mach" from src/tests/lib/libpthread in the NetBSD 7 source:

http://cvsweb.netbsd.org/bsdweb.cgi/src/tests/lib/libpthread/

I've also attached these files to this ticket.*

The test program h_resolv.c compiles fine on Mac OS X 10.10 with clang 7, and the test passes using the same parameters that the NetBSD test uses, resolving five host names on five threads:

./h_resolv -d -n 5 -h 5 d_mach

There's even evidence that getaddrinfo is actually concurrent on my Mac: resolving 100 hostnames on 1 thread takes about 6 seconds, but using 100 threads take only 3 seconds. To test concurrency I ran this:

sudo dscacheutil -flushcache
sudo killall -HUP mDNSResponder
time h_resolv -d -h 100 -n 1 d_mach

sudo dscacheutil -flushcache
sudo killall -HUP mDNSResponder
time h_resolv -d -h 1 -n 100 d_mach

Those sudo commands flush the Mac's DNS cache between runs.

• I removed one unresolvable domain, "cnftp.bjpu.edu.cn", from the test file d_mach. That domain took 15 seconds to time out and made the rest of the timing irrelevant.

[ajdavis]

An Apple Developer Relations engineer tells me it's "reasonable to assume that getaddrinfo() is thread safe" on OS X 10.5 and later. (He mentioned that iOS inherited the OS X 10.5 DNS architecture, so Apple phones, watches, TVs, hairdryers, etc. can all do concurrent DNS too.)

Before OS X 10.5 the DNS system was a mix of new Mac OS X features, mDNSResponder and Open Directory, along with historical lookupd from NeXT and libresolv from an old BSD. In 10.5 the system was cleaned up to depend on OS X's mDNSResponder consistently; in the process getaddrinfo became thread-safe.

He cites as further evidence of its thread safety:

• The "bugs" section of getaddrinfo's man page was removed in 10.5
• 10.5 is the first OS X to be UNIX '03 certified, which includes the requirement that getaddrinfo be thread-safe
• The system itself uses getaddrinfo extensively as if it's thread-safe
• He hasn't seen reports of thread-safety problems with getaddrinfo

He explained how the code works. In the latest version:

http://www.opensource.apple.com/source/Libinfo/Libinfo-476/lookup.subproj/libinfo.c

getaddrinfo calls down to libinfo’s "mdns" module:

http://www.opensource.apple.com/source/Libinfo/Libinfo-476/lookup.subproj/mdns_module.c

"mdns" uses the DNS-SD API declared in dns_sd.h, which he says is "well known to be thread safe." The DNS-SD API is part of the mDNSResponder project:

http://www.opensource.apple.com/source/mDNSResponder/mDNSResponder-576.30.4/

The key function is DNSServiceQueryRecord:

http://www.opensource.apple.com/source/mDNSResponder/mDNSResponder-576.30.4/mDNSShared/dnssd_clientstub.c

My contact concludes, "As you can see, it does an IPC over to the mDNSResponder process, at which point thread safety is assured."

[ajdavis]

I've created a Mac OS 10.4 virtual machine and reproduced the getaddrinfo concurrency bug there using the attached h_resolv.c. The man page on that OS version indeed includes the "not thread-safe" warning.

The same test passes on my Mac OS 10.10 system.

I am convinced that getaddrinfo is thread-safe on Mac OS 10.5+, and I'm attaching a patch to disable the lock on these systems.

[gvanrossum]

Thanks for the thorough work! Hopefully we can apply this fix to 3.5, 3.6 and even 2.7.

[ajdavis]

Great, how do we ensure this gets merged soon?

[gvanrossum]

Find an active developer who cares -- e.g. Martin Panter, Serhiy, Yury.

On Tue, Feb 2, 2016 at 5:46 AM, A. Jesse Jiryu Davis
<report@bugs.python.org> wrote:

A. Jesse Jiryu Davis added the comment:

Great, how do we ensure this gets merged soon?

----------

---

Python tracker <report@bugs.python.org>
<http://bugs.python.org/issue25924\>

---

[1st1]

Hi Jesse, could you please update your patch with a detailed comment summarizing your research? Also it would be great if you can provide a separate patch for 2.7.

[ajdavis]

Second patch with a comment about how we know Mac OS 10.5+'s getaddrinfo is thread-safe. I'll wait for this to be merged before submitting another for Python 2.7.

[vadmium]

Hello Jesse. Maybe Yury might be more qualified here than me to handle OS X stuff (correct me if I’m wrong). I mainly just work with Linux.

But reading around socketmodule.c, it sounds like we want to be able to build it on newer OS versions, but still have it compatible with older versions. If so, how does that MAC_OS_X_VERSION_10_5 macro work? Would some run-time check also be needed?