Fall back to non-threaded DNS requests if can't start async DNS thread?

[NattyNarwhal]

I did this

A lot of IBM i users are calling curl (not the native ILE version, but the Unix version under PASE, though the ILE native one will likely be affected since pthread is the same under both), be it standalone or via say, PHP, through programs/scripts in the native environment. The native environment has... quirky thread safety rules, so by default, programs invoked can't create threads. As such, curl will usually fail out with a message like getaddrinfo() thread failed to start.

I expected the following

I end up telling people a lot to add the magic environment variable or such to make threading work. While this is ideal (you want async DNS to work if it can), it is pretty annoying; I wonder if a fallback is reasonable, or perhaps on i specifically, a better error message?

curl/libcurl version

curl 7.70.0 (powerpc-ibm-os400) libcurl/7.70.0 OpenSSL/1.1.1g zlib/1.2.11 libssh2/1.9.0
Release-Date: 2020-04-29
Protocols: dict file ftp ftps gopher http https imap imaps pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: AsynchDNS HTTPS-proxy IPv6 Largefile libz NTLM NTLM_WB SSL TLS-SRP UnixSockets

operating system

IBM i PASE 7.2

[jay]

Features: AsynchDNS

You built curl for asynchronous DNS which is either c-ares or threaded, and since there is no ares library listed in the libraries that means it is threaded. In other words, you built curl to use threaded dns.

[bagder]

I wonder if a fallback is reasonable

On all other systems that I'm aware of, the creation of a new thread won't just fail and we would be okay with it and instead try another method so this feels like a special need for this platform.

a better error message?

What would a better error message be like? It does fail to start the thread.

[bagder]

This is not actually an "issue" or a "bug" in current curl. This is discussing a future potential improvement or enhancement of name resolving for a particular target. We're always for improvements, but we need someone to write the code and propose it as a pull-request so that can see what it means and eventually merge it.

This particular change is very target-specific and will basically require that someone who runs and uses such a platform works out the implementation details to make sure they get done right.

[NattyNarwhal]

wrt Async enabled: yes, that's how it comes from the downstream, and it's desirable.... except for this corner case.

wrt target specific: in retrospect, thread creation difficulty is pretty rare, so it effectively is a platform-specific problem despite being cross-platform in theory.

The simplest/least intrusive way might be to append an additional message to that error message for platforms like this that suffer from threading confusion. Orr it might just simply fall on people like me who support these users to get them to actually... turn on the threading option. I definitely understand if upstream here isn't willing to take the additional effort here.

[bagder]

I definitely understand if upstream here isn't willing to take the additional effort here.

It's us we're talking about, we are not upstream. We are here.

Again: I would probably be okay with "taking the additional effort" if that could be seen as a way to make life easier for a certain number of users, but someone (else) would have to roll up their sleeves and provide the code because I won't. The code needs to be verified for both scenarios on relevant systems.

[NattyNarwhal]

Sorry there - was thinking way too much of downstreams and I'm a little loopy on holidays.

And agreed on above. A patch that doesn't change behaviour, just explains the solution for affected users would be something like:

diff --git a/lib/asyn-thread.c b/lib/asyn-thread.c
index c453203f7..b9fe5dbb9 100644
--- a/lib/asyn-thread.c
+++ b/lib/asyn-thread.c
@@ -733,7 +733,11 @@ struct Curl_addrinfo *Curl_resolver_getaddrinfo(struct Curl_easy *data,
     return NULL;
   }
 
-  failf(data, "getaddrinfo() thread failed to start");
+  failf(data, "getaddrinfo() thread failed to start"
+#if defined(__OS400__) || defined(__PASE__) /* IBM i ILE, IBM i PASE */
+  " (on IBM i, you may need to set env var QIBM_MULTI_THREADED to Y)"
+#endif
+  );
   return NULL;
 
 }

[bagder]

1. Couldn't a fix rather set that environment variable (with putenv() or similar) ?
2. If not, shouldn't this rather be documented in an OS/400 documentation section?
3. If libcurl should fail if that environment variable isn't set, shouldn't it then rather check for it in the init call and fail at the earliest possible moment?

[NattyNarwhal]

1. I'm pretty sure this has to be set before the job/process is created.
2. I think that's reasonable. As of right now, I have to basically put up a page for this quirk and refer people to it every time they say it's broken (which can be cryptic!), but if curl's own documentation has it, then it'll be easier to find. Possible caveat: The OS/400 documentation I imagine is oriented towards the ILE version, so it might not be the best place for PASE (AIX syscall emulation, for context), edit even though it affects both.
3. I think that might be a more reasonable solution, but AFAIK the environment variable isn't necessarily required. I think there are flags you can set when creating a job, and you can do that when starting the job (people never touch those flags), or done for you, like interactive Unix shells.

[bagder]

I think this problem should be fixed by documentation rather than putting such a platform specific message into the error message. The error message also won't help users, as it is rather an instruction to whoever built curl and I think its the wrong place for such an instruction.

[NattyNarwhal]

Thinking about it over the weekend, I agree. I assume minority platform-specific documentation should go in packages/, per my skimming of docs/ and finding not too much platform-specific there. (However, packages/os400 is again, for the native curl, not AIX curl on os400 which is also affected, so it might be misleading just there)

[bagder]

I think you need to help us suggest where to put it and what to say. The docs/INSTALL.md file could be another option that already features a lot of chapters for system specific info.