Repository navigation
Ensure that multiprocessing.util.get_temp_dir() can be used to create socket files with limited path length #132124
Description
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 tempdirThis 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
Metadata
Metadata
Assignees
Labels
Projects
- StatusShow more project fieldsDone
Activity
picnixz commentedon Apr 5, 2025
Strictly speaking, [1] says (emphasis mine):
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:
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:Now,
d = /path/with/many/chars/pymp-XXXXXXXX. Maybelen(d) < 108butlen(f) > 108. So it still wouldn't be sufficient, so re-generatingdwould 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
$TMPDIRmay make their application fail.picnixz commentedon Apr 5, 2025
Out of curiosity: what would be the use-case?
Poupine commentedon Apr 5, 2025
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 commentedon Apr 5, 2025
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.
[-]multiprocessing.forkserver.Forkserver.ensure_running() *may* crash with OSError: AF_UNIX path too long[/-][+]Possible `OSError: AF_UNIX path too long` in multiprocessing.forkserver.Forkserver.ensure_running()[/+]Poupine commentedon Apr 7, 2025
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:
Or (to keep using the user-specific tmp folder in case it fits within the expected path length):
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 commentedon Apr 7, 2025
POSIX standards dictates that
/tmpshall 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$TMPDIRI think).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 commentedon Apr 7, 2025
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 situationThis 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 commentedon Apr 7, 2025
No this is a contract that the
/tmpdirectory must respect (ref)We will make the assumption that
/tmpis a valid location. If you run Python, then it should be allowed to write to anything designed as the temporary directory.Not really. Creating the socket is necessary for
Forkserverto run. Not wanting a socket but still wanting Forkserver to work is not something Python wants to deal with.I don't think this has anything with being a file or not.
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
$TMPDIRhas been specified.picnixz commentedon Apr 7, 2025
Note: we could change
listener-tolstjust to save a few characters as we're already in a temporary folderpicnixz commentedon Apr 7, 2025
Ok, actually nvm what I said. I apologize but:
So technically speaking... we could say that
/tmpis allowed... But_candidate_tempdir_list()is only used to generate the default temporary directory (used by thetempfile.*functions). So a possible fix is the following: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
$TMPDIRif the latter is not good for multi-processing.Poupine commentedon Apr 8, 2025
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 :)
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.
21 remaining items
pythongh-132124: improve safety nets for creating AF_UNIX socket files (
hugovk commentedon Jul 22, 2025
3.14 RC1 note: this has been fixed for 3.14, the
deferred-blockerlabel is for 3.13.Yhg1s commentedon Aug 4, 2025
Is this ready to go into 3.13 now? (3.13.6 is scheduled for tomorrow.)
picnixz commentedon Aug 4, 2025
Oh, err, I'm leaving tomorrow but I think it should be ready as we didn't have any feedback for the past months.
[3.13] gh-132124: improve safety nets for creating AF_UNIX socket fil…
gpshead commentedon Aug 4, 2025
merged for 3.13.
pythongh-132124: improve safety nets for creating AF_UNIX socket files (
pythongh-132124: improve safety nets for creating AF_UNIX socket files (