glibc deprecated some stuff, leading to warnings

[faho]

In glibc 2.25, a few things we use are marked as deprecated. On my system, this leads to the only remaining compiler warnings.

The full list:

• readdir_r (recommending plain readdir, which is apparently thread-safe as long as no two threads access the same directory stream simultaneously on glibc and solaris - see e.g. readdir_r is deprecated as of glibc-2.24 rust-lang/rust#34668)

• sys_errlist and sys_nerr (recommending strerror or strerror_r)

[faho]

Urgh, the sys_errlist stuff is annoying - see #1830.

[krader1961]

Note that sys_errlist and sys_nerr have been implicitly deprecated for nearly twenty years. It sucks that strerror is not fork safe. If we can't come up with a better solution I recommend calling strerror for all possible errno values and caching the results as soon as we've initialized the environment (in particular the locale subsystem) but before doing anything that would fork.

A solution I've pondered for this type of problem is an internal RPC mechanism. That would allow the child side of a fork to ask a RPC thread in the main fish process (via a fifo or anonymous unix domain socket) to do something that might acquire a lock and get the results back. This would be a non-trivial amount of work to implement but would allow us to simplify the code in other ways such as eliminating debug_safe(). However, doing so in a manner that could be used without fear of deadlock on the child side of a fork is going to be a challenge.

A far simpler solution is to use a barrier to ensure all threads are quiescent before calling fork/spawn. The child process is then free to call malloc, strerror, etc. Yes, I know we generally want to avoid blocking on a thread completing an action. But we have very few threads and none are likely to cause a delay in completing the fork/spawn. If we do have such a thread it is generally trivial to add barrier checks in its inner loop(s) to keep the latency low. The theoretical problem with this approach is that threads created "behind our back" by some library function throws a monkey wrench into the scheme. But we're unlikely to use an API that does so and I think the benefits of this approach outweigh the risks.

[krader1961]

Note that @ridiculousfish has already proposed something similar to my preferred solution in our pthread manifesto:

However, this is certainly a fragile area so bugs here are likely. fish has no long-running threads, and so fish can also be put into a mode where it waits for its threads to exit before calling fork. This should improve safety, but may re-introduce fish 1.x's slowdowns on slow filesystems.

However, note that what I'm proposing does not involve waiting for those threads to exit. I am proposing that we introduce barriers where we block further execution of all threads, other than the main thread, when we need to fork/spawn until the fork/spawn has been done. Thus ensuring that the child process inherits a state that should be free of mutexes held that could result in a deadlock.

[mqudsi]

I haven't looked into glibc's implementation of the XSI-compliant version of strerror_r, but can we just wrap it in a function that obtains a named semaphore, clones the contents to a wcstring, and returns the result? There's no need for it to be fast, it's only used in error-handling code.

[krader1961]

@mqudsi: By "named" semaphore do you mean a mutex that is stored in a page of memory shared by the parent and child process? If no then your solution doesn't work. And if yes then I think my proposal is simpler and more general.

[mqudsi]

@krader1961 I meant using sem_open with a statically allocated guid as the name; but that would only work presuming all uses of the function take place pre-fork/post-exec. If the mutex has already been opened by the parent process at startup, the calls to sem_open and sem_wait should be allocation-free (and refer to the same semaphore).

It probably wouldn't be too hard to wrap all calls to lock/unlock in a function that increments or decrements some global "obtained mutexes" count and have any fork calls block until that value is zero (condition variable + another mutex).

But I have a different question: what's the reason behind the mix of threads and fork? If you limit forking to a single thread (not necessarily the main thread) used to spawn external processes (so never fork without a more-or-less-immediate exec), can you not avoid the issue entirely? I wonder if it would be too difficult to eliminate non-exec forks with threads that call the respective functions and sidestep the issue entirely.

[krader1961]

But I have a different question: what's the reason behind the mix of threads and fork?

I don't know as I've only been contributing since October 2015. @ridiculousfish may have insight into that question. If I had to guess I would hypothesize that it's the result of the original C (not C++) implementation not using threads. Then when it was rewritten in C++ support for threads was added so that features like history based autosuggestions don't block the user input thread. But the thread implementation was done organically to minimize changes to the existing code that spawns jobs.

[faho]

Okay, I now made the readdir change for #5458.