Ensure that `multiprocessing.util.get_temp_dir()` can be used to create socket files with limited path length

[Poupine]

Bug report

Bug description:

I must admit that this issue only happens in very specific conditions, namely when using the multiprocessing lib on a system where the temp folder is set to a long path by the user. One could argue this is a user error, but bear with me a second.
The problem is in multiprocessing.util.get_temp_dir

def get_temp_dir():
    # get name of a temp directory which will be automatically cleaned up
    tempdir = process.current_process()._config.get('tempdir') # This could be set to an arbitrary long path by users
    if tempdir is None:
        import shutil, tempfile
        tempdir = tempfile.mkdtemp(prefix='pymp-')
        info('created temp directory %s', tempdir)
        # keep a strong reference to shutil.rmtree(), since the finalizer
        # can be called late during Python shutdown
        Finalize(None, _remove_temp_dir, args=(shutil.rmtree, tempdir),
                 exitpriority=-100)
        process.current_process()._config['tempdir'] = tempdir
    return tempdir

This function is used in multiprocessing.connect.arbitrary_address

def arbitrary_address(family):
    '''
    Return an arbitrary free address for the given family
    '''
    if family == 'AF_INET':
        return ('localhost', 0)
    elif family == 'AF_UNIX':
        return tempfile.mktemp(prefix='listener-', dir=util.get_temp_dir())
    elif family == 'AF_PIPE':
        return tempfile.mktemp(prefix=r'\\.\pipe\pyc-%d-%d-' %
                               (os.getpid(), next(_mmap_counter)), dir="")
    else:
        raise ValueError('unrecognized family')

Where this is problematic is that arbitrary_address is used in multiprocessing.forkserver.Forkserver.ensure_running which uses the address provided by arbitrary_address to create and bind to a socket

with socket.socket(socket.AF_UNIX) as listener:
    address = connection.arbitrary_address('AF_UNIX')
    listener.bind(address)
    if not util.is_abstract_socket_namespace(address):
        os.chmod(address, 0o600)
    listener.listen()

Since UNIX sockets path have a limited character count between 92 and 108 [1], it would make sense for the std lib to have safe guards against generating paths that are too long to begin with. Sure, users can use work arounds such as setting TMP_DIR="/tmp" [2] but this requires each and every user of the multiprocessing library to be aware of such limitations.

I propose we fix the problem once and for all at the source, by checking the size of the path created by tempfile.mktemp(prefix='listener-', dir=util.get_temp_dir()) and should it be greater or equal to the max length supported by the platform, reverting to what get_temp_dir() does when tempdir=None

[1] https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_un.h.html#tag_13_67_04
[2] https://patchwork.ozlabs.org/project/qemu-devel/patch/20220722182508.89761-2-peter@pjd.dev/#2938322

CPython versions tested on:

3.11

Operating systems tested on:

Linux

Linked PRs

• gh-132124: improve safety nets for creating AF_UNIX socket files #134085
• [3.14] gh-132124: improve safety nets for creating AF_UNIX socket files (GH-134085) #134447
• [3.13] gh-132124: improve safety nets for creating AF_UNIX socket files (GH-134085) #134448

[picnixz]

Strictly speaking, [1] says (emphasis mine):

Since most implementations originate from BSD versions, the size is typically in the range 92 to 108.

This is not a strict requirement. On Linux it's 108 however. But now, let's say someone is deliberatly using a very long path for their temporary directory by setting $TMPDIR. In this case, they will likely have other issues as well with other applications, not just Python IMO.

Now, at the C level we do:

if ((size_t)path.len > sizeof addr->sun_path) {
    PyErr_SetString(PyExc_OSError,
                    "AF_UNIX path too long");
    goto unix_out;
}

so the error you see is actually an explicit error, not something coming from the kernel. It's in particular adaptive, in the sense that it depends on the underly implementation (we don't hardcode it as 108 for instance).

---

Now let's say we want to fix this by falling back to the default case of get_temp_dir(). I fail to see how this would solve things as we the path would still be using the very long temporary directory defined by the user (namely $TMPDIR).

Note that process.current_process()._config['tempdir'] should NOT be set by users, it's a private entry and only used as a cache. So the only way to change it is to first change $TMPDIR, e.g:

# before: export TMPDIR="/path/with/many/chars"
d = util.get_temp_dir()
f = tempfile.mktemp(prefix='listener-', dir=d)

Now, d = /path/with/many/chars/pymp-XXXXXXXX. Maybe len(d) < 108 but len(f) > 108. So it still wouldn't be sufficient, so re-generating d would not matter as temporary files and directories have the same number of characters by default (Python's mktemp always uses 8-chars random names)

In conclusion, I don't think we can do something on our side, except possibly documenting that users should be aware that a very long $TMPDIR may make their application fail.

[picnixz]

Out of curiosity: what would be the use-case?

[Poupine]

Actually, I take back what I said on the default case of get_temp_dir. I misread the code and thought it would use some sensible default (like e.g. simply /tmp) but I see now that it will simply use the insanely long temp folder provided by the user.

I assume defaulting to /tmp is only a sensible thing to do in 99.9% of cases but will likely cause some weird unexpected problem for someone somewhere? In which case, better documentation would definitely help.

As for the use-case: this is an issue I encountered while using multiprocessing in a pipeline I wrote at Google where the system happens to have a big ass TMPDIR (not set by me) and there has been an issue open for almost 5 years now. Of course, lots of comments on the issue simply mentioned to overwrite the system auto-generated temp dir but I wanted to see if we could fix this issue at the root, by checking the path and falling back to a sensible default so as to fix it once and for all.

Maybe there is scope for not even relying on a user-editable environment variable (TMPDIR in this case) at all? After all, the code does hint at the fact that it does not care where the listener lives (the function is named arbitrary_address), it just needs to exist somewhere under a path that is not too long :)

[terryjreedy]

I agree that not having success depend like this on an external environmental would be preferable, but I don't know what a dependable alternative would be. That other apps may also fail is not our direct concern.

Can you identify a place where the doc could be changed? The problem seems pretty remote from any user code.

Is the quoted code from the main branch or from 3.11, which might have changed? As the issue title, we use 'crash' here on the tracker for core-dumps, etcetera, and not Python exception exits.

[Poupine]

I tested only on 3.11 but looking at the code on main, I don't think it has changed.

I'll look into a place in the doc where this could be added, but as you @terryjreedy, this happens rather far from user code, so I'm not sure this is the best way forward

Re: "I agree that not having success depend like this on an external environmental would be preferable, but I don't know what a dependable alternative would be. That other apps may also fail is not our direct concern."

What comes to mind is either of the follwoing:

def arbitrary_address(family):
    '''
    Return an arbitrary free address for the given family
    '''
    ...
    elif family == 'AF_UNIX':
        return tempfile.mktemp(prefix='listener-', dir='/tmp')
    ...

Or (to keep using the user-specific tmp folder in case it fits within the expected path length):

def arbitrary_address(family):
    '''
    Return an arbitrary free address for the given family
    '''
   ...
    elif family == 'AF_UNIX':
        listener_path = tempfile.mktemp(prefix='listener-', dir=util.get_temp_dir())
        if len(listener_path) > THRESHOLD:
            listener_path = tempfile.mktemp(prefix='listener-', dir='/tmp')
        return listener_path
    ...

THRESHOLD would need to be determined based on the system (or hard coded to 92?). In any case, having '/tmp' instead of a user specified temp folder should work on most UNIX systems?

[picnixz]

In any case, having '/tmp' instead of a user specified temp folder should work on most UNIX systems?

POSIX standards dictates that /tmp shall exist (https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap10.html#tag_10_01). But I don't really know whether it's a good idea to fall back to /tmp. Maybe someone doesn't want Python to write anything out there? (well, that's why they would set their $TMPDIR I think).

I wrote at Google where the system happens to have a big ass TMPDIR (not set by me)

Honestly, if the path is already very large, it's not only Python that will be affected but any other application that would create socket files. So I'm not really keen on changing Python to accomodate this specific situation. I'll leave the decision to @gpshead as he's the mp-maintainer. Personally, the best I can suggest is to improve the docs.

[Poupine]

But I don't really know whether it's a good idea to fall back to /tmp. Maybe someone doesn't want Python to write anything out there ?

The same argument could be used against creating a socket in the first place. There is no guarantee that the user actually wants anything to be written by python in the temp folder they provided. But then again, there is no way around that since ultimately everything is a file on UNIX. My guess is most users don't even know (or care) about how this is all implemented under the hood.

So I'm not really keen on changing Python to accomodate this specific situation

This is not a specific situation though. Sure, it's something I personally ran into at Google, I've seen this issue pop up over again in various projects. See here for example:
https://patchwork.ozlabs.org/project/qemu-devel/patch/20220722182508.89761-2-peter@pjd.dev/#2938322
#93852
psf/black#2105
https://git-r3lab.uni.lu/IMP/imp3/-/issues/54

[picnixz]

There is no guarantee that the user actually wants anything to be written by python in the temp folder they provided

No this is a contract that the /tmp directory must respect (ref)

A directory made available for applications that need a place to create temporary files. Applications shall be allowed to create files in this directory, but shall not assume that such files are preserved between invocations of the application.

We will make the assumption that /tmp is a valid location. If you run Python, then it should be allowed to write to anything designed as the temporary directory.

The same argument could be used against creating a socket in the first place

Not really. Creating the socket is necessary for Forkserver to run. Not wanting a socket but still wanting Forkserver to work is not something Python wants to deal with.

But then again, there is no way around that since ultimately everything is a file on UNIX.

I don't think this has anything with being a file or not.

I've seen this issue pop up over again in various projects. See here for example:

AFAICT, what users want to know is why they have this issue. Agreed that the execption is clearly not helpful if you don't know what happens under the hood, so we could improve it (e.g., by showing what kind of file we're trying to create or by saying that we want to create a socket file [the exception is raised from the socketmodule])

But again, I really don't want to switch to another directory if an explicit $TMPDIR has been specified.

[picnixz]

Note: we could change listener- to lst just to save a few characters as we're already in a temporary folder

[picnixz]

Ok, actually nvm what I said. I apologize but:

def _candidate_tempdir_list():
    """Generate a list of candidate temporary directories which
    _get_default_tempdir will try."""

    dirlist = []

    # First, try the environment.
    for envname in 'TMPDIR', 'TEMP', 'TMP':
        dirname = _os.getenv(envname)
        if dirname: dirlist.append(dirname)

    # Failing that, try OS-specific locations.
    if _os.name == 'nt':
        dirlist.extend([ _os.path.expanduser(r'~\AppData\Local\Temp'),
                         _os.path.expandvars(r'%SYSTEMROOT%\Temp'),
                         r'c:\temp', r'c:\tmp', r'\temp', r'\tmp' ])
    else:
        dirlist.extend([ '/tmp', '/var/tmp', '/usr/tmp' ])

    # As a last resort, the current directory.
    try:
        dirlist.append(_os.getcwd())
    except (AttributeError, OSError):
        dirlist.append(_os.curdir)

   return dirlist

So technically speaking... we could say that /tmp is allowed... But _candidate_tempdir_list() is only used to generate the default temporary directory (used by the tempfile.* functions). So a possible fix is the following:

tempdir = tempfile.mkdtemp(prefix='pymp-')
# check if tempdir will be small enough to accomodate the future socket files
# if not: 
tempdir = tempfile.mkdtemp(prefix='pymp-', dir='/tmp')
# no check at all

It could mitigate the issue without having to change error messages and without changing the existing behaviour. However, this should be documented as we would ignore any supplied $TMPDIR if the latter is not good for multi-processing.

[Poupine]

A directory made available for applications that need a place to create temporary files. Applications shall be allowed to create files in this directory, but shall not assume that such files are preserved between invocations of the application.

TIL :)

tempdir = tempfile.mkdtemp(prefix='pymp-')
# check if tempdir will be small enough to accomodate the future socket files
# if not: 
tempdir = tempfile.mkdtemp(prefix='pymp-', dir='/tmp')
# no check at all

Yes, something like this is what I had in mind. Let's wait for @gpshead to comment and see if he is okay with this change.