[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid -> process mapping.

[Benjamin Peterson]

Benjamin Peterson  added the comment:

Yes

On Wed, Oct 31, 2018, at 22:40, Mike Frysinger wrote:
>
> Mike Frysinger  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 
> 
> ___

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid -> process mapping.

[Mike Frysinger]

Mike Frysinger  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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid -> process mapping.

[Benjamin Peterson]

Benjamin Peterson  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid -> process mapping.

[Benjamin Peterson]

Change by Benjamin Peterson :


--
pull_requests: +9590

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid -> process mapping.

[Mike Frysinger]

Mike Frysinger  added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid -> process mapping.

[Ryan Petrello]

Ryan Petrello  added the comment:

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

--
nosy: +ryan.petrello

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid -> process mapping.

[Davin Potts]

Davin Potts added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid -> process mapping.

[Roundup Robot]

Roundup Robot added the comment:

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

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid -> process mapping.

[Mike Frysinger]

Mike Frysinger added the comment:

i don't feel strongly about either version

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid -> process mapping.

[Charles-François Natali]

Charles-François Natali added the comment:

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Charles-François Natali]

Changes by Charles-François Natali cf.nat...@gmail.com:


--
keywords: +needs review
nosy: +haypo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Charles-François Natali]

Charles-François Natali added the comment:

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

--
keywords: +patch
nosy: +neologix
versions: +Python 3.3
Added file: http://bugs.python.org/file39679/mp_sem_race.diff

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Mike Frysinger]

Mike Frysinger added the comment:

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 ?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Mike Frysinger]

Mike Frysinger added the comment:

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!
real0m1.715s

--
Added file: http://bugs.python.org/file39594/test-issue24303.py

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Davin Potts]

Davin Potts added the comment:

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.

--
assignee:  - davin
components: +Library (Lib)
type: enhancement - behavior

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Yury Selivanov]

Changes by Yury Selivanov yseliva...@gmail.com:


--
nosy: +jnoller

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Davin Potts]

Davin Potts added the comment:

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?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Mike Frysinger]

Mike Frysinger added the comment:

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

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Dirkjan Ochtman]

Changes by Dirkjan Ochtman dirk...@ochtman.nl:


--
nosy: +djc

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Paul Hobbs]

New submission from Paul Hobbs:

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;

--
messages: 244211
nosy: Paul Hobbs
priority: normal
severity: normal
status: open
title: OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid 
- process mapping.
type: enhancement
versions: Python 2.7

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Ned Deily]

Changes by Ned Deily n...@acm.org:


--
nosy: +devin, sbt
stage:  - patch review

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Ned Deily]

Changes by Ned Deily n...@acm.org:


--
nosy: +davin -devin

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Mike Frysinger]

Changes by Mike Frysinger vap...@users.sourceforge.net:


--
nosy: +vapier

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Arfrever Frehtes Taifersar Arahesis]

Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com:


--
nosy: +Arfrever

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue24303] OSError 17 due to _multiprocessing/semaphore.c assuming a one-to-one Pid - process mapping.

[Mike Gilbert]

Changes by Mike Gilbert floppymas...@gmail.com:


--
nosy: +floppymaster

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24303
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com