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

lib/hostip.c: SCDynamicStoreCopyProxies is not safe to run in a fork #11252

Closed
stanhu opened this issue Jun 5, 2023 · 8 comments
Closed

lib/hostip.c: SCDynamicStoreCopyProxies is not safe to run in a fork #11252

stanhu opened this issue Jun 5, 2023 · 8 comments

Comments

@stanhu
Copy link
Contributor

stanhu commented Jun 5, 2023

I did this

The Elasticsearch Ruby client uses the Typhoeus Ruby gem, which uses libcurl. If you do something like this:

require 'typhoeus'

fork {
  Typhoeus.head('https://wwww.google.com')
}

The process crashes with this warning:

objc[73067]: +[__NSCFConstantString initialize] may have been in progress in another thread when fork() was called.
objc[73067]: +[__NSCFConstantString initialize] may have been in progress in another thread when fork() was called. We cannot safely call it or ignore it in the fork() child process. Crashing instead. Set a breakpoint on objc_initializeAfterForkError to debug.

I expected the following

No error.

For simple applications, you can work around the problem by making the libcurl call outside of the fork:

require 'typhoeus'
 
Typhoeus.head('http://localhost')

fork {
  Typhoeus.head('http://localhost')
}

However, this doesn't always work. As discussed in the links below, these macOS system calls are not thread-safe. Under some circumstances calling SCDynamicStoreCopyProxiesWithOptions can crash with a backtrace as the following:

Target 0: (ruby) stopped.
(lldb) bt
* thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x109be0b22)
  * frame #0: 0x0000000186ed66a0 libsystem_trace.dylib`_os_log_preferences_refresh + 64
    frame #1: 0x0000000186ed710c libsystem_trace.dylib`os_log_type_enabled + 712
    frame #2: 0x00000001871ea430 CoreFoundation`-[CFPrefsSearchListSource alreadylocked_copyValueForKey:] + 204
    frame #3: 0x00000001871ea344 CoreFoundation`-[CFPrefsSource copyValueForKey:] + 52
    frame #4: 0x00000001871ea2f8 CoreFoundation`__76-[_CFXPreferences copyAppValueForKey:identifier:container:configurationURL:]_block_invoke + 32
    frame #5: 0x00000001871e3260 CoreFoundation`__108-[_CFXPreferences(SearchListAdditions) withSearchListForIdentifier:container:cloudConfigurationURL:perform:]_block_invoke + 376
    frame #6: 0x000000018735fc78 CoreFoundation`-[_CFXPreferences withSearchListForIdentifier:container:cloudConfigurationURL:perform:] + 384
    frame #7: 0x00000001871e2b30 CoreFoundation`-[_CFXPreferences copyAppValueForKey:identifier:container:configurationURL:] + 168
    frame #8: 0x00000001871e2a4c CoreFoundation`_CFPreferencesCopyAppValueWithContainerAndConfiguration + 112
    frame #9: 0x0000000187e23f24 SystemConfiguration`SCDynamicStoreCopyProxiesWithOptions + 180
    frame #10: 0x000000019d29c01c libcurl.4.dylib`Curl_resolv + 288
    frame #11: 0x000000019d2cd40c libcurl.4.dylib`resolve_server + 320
    frame #12: 0x000000019d2cba80 libcurl.4.dylib`Curl_connect + 5764
    frame #13: 0x000000019d2b3344 libcurl.4.dylib`multi_runsingle + 580
    frame #14: 0x000000019d2b2fdc libcurl.4.dylib`curl_multi_perform + 124
    frame #15: 0x000000019d290bcc libcurl.4.dylib`curl_easy_perform + 276
    frame #16: 0x0000000111a68044 ffi_c.bundle`ffi_call_SYSV + 68
    frame #17: 0x0000000111a639fc ffi_c.bundle`ffi_call_int + 1560
    frame #18: 0x0000000111a633d8 ffi_c.bundle`ffi_call + 52
    frame #19: 0x0000000111a56ef0 ffi_c.bundle`call_blocking_function(data=<unavailable>) at Call.c:336:5
    frame #20: 0x0000000105167cd0 libruby.3.1.dylib`rb_nogvl + 268
    frame #21: 0x0000000111a56ec8 ffi_c.bundle`rbffi_do_blocking_call(data=<unavailable>) at Call.c:344:5
    frame #22: 0x0000000105019af8 libruby.3.1.dylib`rb_vrescue2 + 368
    frame #23: 0x0000000105019960 libruby.3.1.dylib`rb_rescue2 + 44
    frame #24: 0x0000000111a5715c ffi_c.bundle`rbffi_CallFunction(argc=<unavailable>, argv=<unavailable>, function=0x000000019d290ab8, fnInfo=0x00006000037bad60) at Call.c:387:9
    frame #25: 0x0000000111a5ace8 ffi_c.bundle`attached_method_invoke(cif=<unavailable>, mretval=0x000000016b2cdc90, parameters=<unavailable>, user_data=<unavailable>) at MethodHandle.c:174:26
    frame #26: 0x0000000111a63f08 ffi_c.bundle`ffi_closure_SYSV_inner + 988
    frame #27: 0x0000000111a681b4 ffi_c.bundle`.Ldo_closure + 20
    frame #28: 0x00000001051b64e4 libruby.3.1.dylib`vm_call_cfunc_with_frame + 232
    frame #29: 0x00000001051b8c08 libruby.3.1.dylib`vm_sendish + 1336
    frame #30: 0x000000010519b53c libruby.3.1.dylib`vm_exec_core + 8128
  1. https://blog.phusion.nl/2017/10/13/why-ruby-app-servers-break-on-macos-high-sierra-and-what-can-be-done-about-it/
  2. https://bugs.python.org/issue33725

This system call was introduced in #7121 by @zajdee. I'm wondering: can we avoid calling this system call more than once? For example, I'm wondering if we can move this into curl_global_init() instead.

curl/libcurl version

curl 7.88.1 (x86_64-apple-darwin22.0) libcurl/7.88.1 (SecureTransport) LibreSSL/3.3.6 zlib/1.2.11 nghttp2/1.51.0
Release-Date: 2023-02-20
Protocols: dict file ftp ftps gopher gophers http https imap imaps ldap ldaps mqtt pop3 pop3s rtsp smb smbs smtp smtps telnet tftp
Features: alt-svc AsynchDNS GSS-API HSTS HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz MultiSSL NTLM NTLM_WB SPNEGO SSL threadsafe UnixSockets

operating system

macOS Ventura 13.4

stanhu added a commit to stanhu/curl that referenced this issue Jun 5, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

Closes curl#11252
@zajdee
Copy link

zajdee commented Jun 5, 2023

We could move it to a global initialization function if that's what makes it safer.

stanhu added a commit to stanhu/curl that referenced this issue Jun 5, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

Closes curl#11252
stanhu added a commit to stanhu/curl that referenced this issue Jun 5, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

Closes curl#11252
stanhu added a commit to stanhu/curl that referenced this issue Jun 5, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

Closes curl#11252
stanhu added a commit to stanhu/curl that referenced this issue Jun 5, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

Closes curl#11252
@jay
Copy link
Member

jay commented Jun 5, 2023

Isn't this a forking issue, not a libcurl issue? I don't think we can guarantee forking behavior. See also #788 #6968

@stanhu
Copy link
Contributor Author

stanhu commented Jun 5, 2023

Ultimately it is a forking issue, but I'd argue that #11254 would be useful anyways because:

  1. It avoids extra macOS system calls for every IP lookup.
  2. Consolidates macOS-specific initialization in a separate file/function.

stanhu added a commit to stanhu/curl that referenced this issue Jun 9, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

In addition, this change is beneficial because it:

1. Avoids extra macOS system calls for every IP lookup.
2. Consolidates macOS-specific initialization in a separate file.

Closes curl#11252
@bagder bagder closed this as completed in c730859 Jul 9, 2023
bch pushed a commit to bch/curl that referenced this issue Jul 19, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

In addition, this change is beneficial because it:

1. Avoids extra macOS system calls for every IP lookup.
2. Consolidates macOS-specific initialization in a separate file.

Fixes curl#11252
Closes curl#11254
ptitSeb pushed a commit to wasix-org/curl that referenced this issue Sep 25, 2023
curl#7121 introduced a macOS system call
to `SCDynamicStoreCopyProxies`, which is invoked every time an IP
address needs to be resolved.

However, this system call is not thread-safe, and macOS will kill the
process if the system call is run first in a fork. To make it possible
for the parent process to call this once and prevent the crash, only
invoke this system call in the global initialization routine.

In addition, this change is beneficial because it:

1. Avoids extra macOS system calls for every IP lookup.
2. Consolidates macOS-specific initialization in a separate file.

Fixes curl#11252
Closes curl#11254
@tristan957
Copy link

tristan957 commented Jan 23, 2024

The call to SCDynamicStoreCopyProxies leads to undefined behavior in Postgres (stick with me). Postgres is not a multithreaded program. It is multiprocess using fork(2). Each connection to the server is its own process (backend) which is created by a parent process called the postmaster.

Postgres has a setting called shared_preload_libraries. These are a list of libraries that are dlopened by the postmaster and initialized by calling a _PG_init(void) function. This is seemingly a great place to initialize libcurl for Postgres extensions. Unfortunately, this creates two issues in Postgres related to signal masking and the eventual forks. The signal masking can be worked around via pthread_sigmask() instead of sigprocmask(), which I will bring up on the list.

But the issue with fork(2) is harder to fix. When a fork occurs, it only copies data from the calling thread. What this means for Postgres extensions using libcurl on Mac, is that any data initialized by SCDynamicStoreCopyProxies is no longer initialized.

Then I thought, what if I create a hook for extensions to run code when backends are at steady-state. I could call curl_global_init() in every backend that starts up, which would cause SCDynamicStoreCopyProxies to be called and always have initialized data. Unfortunately, curl_global_init() short-circuits if you call it multiple times, not calling SCDynamicStoreCopyProxies.

At this point, I am currently out of ideas. Do y'all have any suggestions to work-around this terrible situation (where Apple is wholly to blame, not curl developers).

@stanhu
Copy link
Contributor Author

stanhu commented Jan 23, 2024

I'm not quite sure what you're trying to do. Are you trying to write a PostgreSQL extension that loads libcurl on macOS?

@tristan957
Copy link

Yes. Let me edit my message

@stanhu
Copy link
Contributor Author

stanhu commented Jan 23, 2024

This sounds more like a question for the PostgreSQL mailing list, but I thought that the main process (postmaster) first attempts to load all shared libraries: https://github.com/postgres/postgres/blob/29275b1d177096597675b5c6e7e7c9db2df8f4df/src/backend/postmaster/postmaster.c#L1016

Once that server starts up, then the fork() occurs: https://github.com/postgres/postgres/blob/29275b1d177096597675b5c6e7e7c9db2df8f4df/src/backend/postmaster/postmaster.c#L1773-L1776

I would expect that these forked process would have the right state if curl_global_init() were called first.

@tristan957
Copy link

Postgres has a check for pthread_is_threaded_np() here: https://github.com/postgres/postgres/blob/29275b1d177096597675b5c6e7e7c9db2df8f4df/src/backend/postmaster/postmaster.c#L1429-L1445.

I was reading this SO answer (https://stackoverflow.com/a/42679479/7572728) to understand the problem-space a little bit better. It sounds like there is a potential for problems, which may not exist in the context of this particular situation, but could in others.

Anyway, I'll take this to the Postgres list. Thanks for your help :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

4 participants