Using pid namespacing it is possible to have multiple processes with the same pid.  "semlock_new" creates a semaphore file with the template "/dev/shm/mp{pid}-{counter}".  This can conflict if the same semaphore file already exists due to another Python process have the same pid.

This bug has been fixed in Python3: https://bugs.python.org/issue8713.  However, that patch is very large (40 files, ~4.4k changed lines) and only incidentally fixes this bug while introducing a large backwards-incompatible refactoring and feature addition.

The following small patch to just _multiprocessing/semaphore.c fixes the problem by using the system clock and retrying to avoid conflicts:

--- a/Modules/_multiprocessing/semaphore.c
+++ b/Modules/_multiprocessing/semaphore.c
@@ -7,6 +7,7 @@
  */
 
 #include "multiprocessing.h"
+#include <time.h>
 
 enum { RECURSIVE_MUTEX, SEMAPHORE };
 
@@ -419,7 +420,7 @@ semlock_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
 {
     char buffer[256];
     SEM_HANDLE handle = SEM_FAILED;
-    int kind, maxvalue, value;
+    int kind, maxvalue, value, try;
     PyObject *result;
     static char *kwlist[] = {"kind", "value", "maxvalue", NULL};
     static int counter = 0;
@@ -433,10 +434,24 @@ semlock_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
         return NULL;
     }
 
-    PyOS_snprintf(buffer, sizeof(buffer), "/mp%ld-%d", (long)getpid(), counter++);
+    /* With pid namespaces, we may have multiple processes with the same pid.
+     * Instead of relying on the pid to be unique, we use the microseconds time
+     * to attempt to a unique filename. */
+    for (try = 0; try < 100; ++try) {
+        struct timespec tv;
+        long arbitrary = clock_gettime(CLOCK_REALTIME, &tv) ? 0 : tv.tv_nsec;
+        PyOS_snprintf(buffer, sizeof(buffer), "/mp%ld-%d-%ld",
+                      (long)getpid(),
+                      counter++,
+                      arbitrary);
+        SEM_CLEAR_ERROR();
+        handle = SEM_CREATE(buffer, value, maxvalue);
+        if (handle != SEM_FAILED)
+            break;
+        else if (errno != EEXIST)
+            goto failure;
+    }
 
-    SEM_CLEAR_ERROR();
-    handle = SEM_CREATE(buffer, value, maxvalue);
     /* On Windows we should fail if GetLastError()==ERROR_ALREADY_EXISTS */
     if (handle == SEM_FAILED || SEM_GET_LAST_ERROR() != 0)
         goto failure;

At first blush it does appear there is potential for conflict because of how the semaphore filename template was implemented -- that's a cool find.  In practice, I wonder how often this has actually bitten anyone in the real world.  The Linux world's use of clone() (creating pid namespaces) is relatively new/young.  The BSD world's use of jails (bsd-style) take a bit of a different approach, advocate against the use of shared filesystems across jails where a similar conflict could arise, and have been around longer.

@Paul:  Out of curiosity, what inspired your discovery?

Agreed that backporting Richard's work from issue8713 does not appeal.

A few concerns:
* Retrying with a modified filename makes sense but basing it on a timestamp from the system clock is not particularly robust given that the cloned processes can be executing in sufficiently close lock step that they both get the same timestamp (given the granularity/precision of the clock functions).  Developers new to high performance computing often learn this the hard way when trying to use the system clock to create uniquely named files.
* Instead, what about using the system's available pseudo-random number generators?  Most are implemented to avoid just this problem where two or more processes/threads ask for a random/distinct value at nearly the same moment.
* What about simply using a counter (not the same static int counter but another) and incrementing it when the attempt to create the semaphore file fails?  This avoids a system function call and potentially simplifies things.  Would this be faster in the majority of cases?

we hit this problem daily in Chromium OS's build farm because we use pid namespaces heavily

Triggering it regularly in a build farm indeed sounds like genuine pain.

@Paul or @vapier: In tracking down this issue, did you already create a convenient way to repro the misbehavior that could be used in testing?  Any finalized patch that we make will need some form of test.

the original report on our side w/bunches of tracebacks: http://crbug.com/481223

with that traceback in hand, it's pretty trivial to write a reproduction :).  attached!

i couldn't figure out how to make it work w/out completely execing a new python instance.  i suspect the internals were communicating and thus defeating the race.

the unshare() might need some checks so that it skips on older kernels or ones with pidns support disabled.  but i don't have any such system anymore ;).

if all goes well, it should fail fairly quickly:
$ python3.3 ./test.py 
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/lib64/python3.3/multiprocessing/__init__.py", line 187, in Event
    return Event()
  File "/usr/lib64/python3.3/multiprocessing/synchronize.py", line 293, in __init__
    self._cond = Condition(Lock())
  File "/usr/lib64/python3.3/multiprocessing/synchronize.py", line 174, in __init__
    self._wait_semaphore = Semaphore(0)
  File "/usr/lib64/python3.3/multiprocessing/synchronize.py", line 84, in __init__
    SemLock.__init__(self, SEMAPHORE, value, SEM_VALUE_MAX)
  File "/usr/lib64/python3.3/multiprocessing/synchronize.py", line 48, in __init__
    sl = self._semlock = _multiprocessing.SemLock(kind, value, maxvalue)
FileExistsError: [Errno 17] File exists
failed

and doesn't take that long to pass:
$ time python3.4 ./test-issue24303.py 
passed!
real    0m1.715s

also, it looks like this bug is in python-3.3.  not sure what the status of that branch is, but maybe the backport from 3.4 would be easy ?

I think it would be a better idea to partially backport the 2.7 logic that uses the random module.

Here's a patch against 2.7 using _PyOS_URandom(): it should apply as-is to 3.3.

Anyone opposed to me committing the patch I submitted?
It slves a real problem, and is fairly straight-forward (and conceptually more correct).

i don't feel strongly about either version

@neologix: I second your proposed patch -- looks like a winner to me.  Apologies for not following up earlier.

New changeset d3662c088db8 by Charles-François Natali in branch '2.7':
Issue #24303: Fix random EEXIST upon multiprocessing semaphores creation with
https://hg.python.org/cpython/rev/d3662c088db8

Any chance this patch was every applied to Python3?  It looks to me like 3.6 has the old code:

https://github.com/python/cpython/blob/3.6/Modules/_multiprocessing/semaphore.c#L448

it does seem like the patch was never applied to any python 3 branch :/

The retry logic is implemented at a different layer in Python 3: https://github.com/python/cpython/blob/a1c249c40517917d2e0971d55aea8d14a44b2cc8/Lib/multiprocessing/synchronize.py#L115-L117

that's highlighting the SemLock._make_name func which doesn't have retry logic in it.  did you mean to highlight SemLock.__init__ which has a retry loop that looks similar to the C code ?
https://github.com/python/cpython/blob/a1c249c40517917d2e0971d55aea8d14a44b2cc8/Lib/multiprocessing/synchronize.py#L55-L65

Yes

On Wed, Oct 31, 2018, at 22:40, Mike Frysinger wrote:
>
> Mike Frysinger <vapier@users.sourceforge.net> added the comment:
>
> that's highlighting the SemLock._make_name func which doesn't
> have retry> logic in it.  did you mean to highlight SemLock.__init__ which has a
> retry loop that looks similar to the C code ?
> https://github.com/python/cpython/blob/a1c249c40517917d2e0971d55aea8d14a44b2cc8/Lib/multiprocessing/synchronize.py#L55-L65>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue24303>
> _______________________________________