Charles-François Natali neolo...@free.fr added the comment:
The test passes on all the buildbots, closing.
greg, thanks for reporting this!
--
resolution: - fixed
stage: - committed/rejected
status: open - closed
___
Python tracker
Charles-François Natali neolo...@free.fr added the comment:
Updated patch.
--
Added file: http://bugs.python.org/file22545/heap_gc_deadlock_lockless.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12352
Changes by Charles-François Natali neolo...@free.fr:
Added file: http://bugs.python.org/file22546/heap_gc_deadlock_lockless.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12352
___
Changes by Charles-François Natali neolo...@free.fr:
Removed file: http://bugs.python.org/file22477/heap_gc_deadlock.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12352
___
Changes by Charles-François Natali neolo...@free.fr:
Removed file: http://bugs.python.org/file22490/heap_gc_deadlock_lockless.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12352
___
Changes by Charles-François Natali neolo...@free.fr:
Removed file: http://bugs.python.org/file22545/heap_gc_deadlock_lockless.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12352
___
STINNER Victor victor.stin...@haypocalc.com added the comment:
The last heap_gc_deadlock_lockless.diff looks good.
Note: please try to use different filenames for different versions of the same
patch. For example, add a number (heap_gc_deadlock_lockless-2.diff) to the name.
--
Antoine Pitrou pit...@free.fr added the comment:
+if gc.isenabled():
+thresholds = gc.get_threshold()
+self.addCleanup(gc.set_threshold, *thresholds)
+else:
+gc.enable()
+self.addCleanup(gc.disable)
It seems you won't restore the
Roundup Robot devnull@devnull added the comment:
New changeset 96a0788583c6 by Charles-François Natali in branch '2.7':
Issue #12352: Fix a deadlock in multiprocessing.Heap when a block is freed by
http://hg.python.org/cpython/rev/96a0788583c6
--
nosy: +python-dev
Roundup Robot devnull@devnull added the comment:
New changeset 874143242d79 by Charles-François Natali in branch '2.7':
Issue #12352: In test_free_from_gc(), restore the GC thresholds even if the GC
http://hg.python.org/cpython/rev/874143242d79
--
Roundup Robot devnull@devnull added the comment:
New changeset 0d4ca1e77205 by Charles-François Natali in branch '3.1':
Issue #12352: Fix a deadlock in multiprocessing.Heap when a block is freed by
http://hg.python.org/cpython/rev/0d4ca1e77205
--
___
Roundup Robot devnull@devnull added the comment:
New changeset 37606505b227 by Charles-François Natali in branch '3.2':
Merge issue #12352: Fix a deadlock in multiprocessing.Heap when a block is
http://hg.python.org/cpython/rev/37606505b227
--
___
Roundup Robot devnull@devnull added the comment:
New changeset fd8dc3746992 by Charles-François Natali in branch 'default':
Merge issue #12352: Fix a deadlock in multiprocessing.Heap when a block is
http://hg.python.org/cpython/rev/fd8dc3746992
--
Charles-François Natali neolo...@free.fr added the comment:
Nice work! I also think heap_gc_deadlock_lockless.diff is good, except for
Victor's reservation: is it deliberate that you reversed the following two
statements in _free_pending_blocks(), compared to the code in free()?
+
Antoine Pitrou pit...@free.fr added the comment:
Nice work! I also think heap_gc_deadlock_lockless.diff is good, except for
Victor's reservation: is it deliberate that you reversed the following two
statements in _free_pending_blocks(), compared to the code in free()?
+
STINNER Victor victor.stin...@haypocalc.com added the comment:
heap_gc_deadlock_lockless.diff: _free_pending_blocks() and free() execute the
following instructions in a different order, is it a problem?
+self._free(block)
+self._allocated_blocks.remove(block)
vs
+
STINNER Victor victor.stin...@haypocalc.com added the comment:
You may document that _pending_free_blocks.append()
and _pending_free_blocks.pop() are atomic in CPython
and don't need a specific lock.
Oops, i skipped complelty your long comment explaining everything! It is enough.
--
STINNER Victor victor.stin...@haypocalc.com added the comment:
There are different technics to workaround this issue. My preferred is
heap_gc_deadlock_lockless.diff because it has less border effect and have a
well defined behaviour.
--
___
Python
Charles-François Natali neolo...@free.fr added the comment:
[...]
I don't like touching such global variable, but you are right.
Well, I don't like it either, but I can't really think of any other solution.
Antoine, any thought on that?
--
___
Charles-François Natali neolo...@free.fr added the comment:
You are probably right. Can't we use a lock-less list? list.append is
atomic thanks to the GIL, isn't it? But I don't know how to implement
the lock-less list consumer. It would be nice to have a function to
remove and return the
Charles-François Natali neolo...@free.fr added the comment:
Here's a patch based on the second approach.
--
Added file: http://bugs.python.org/file22490/heap_gc_deadlock_lockless.diff
___
Python tracker rep...@bugs.python.org
Charles-François Natali neolo...@free.fr added the comment:
Patch (with test) attached. It disables the garbage collector inside
critical sections.
Of course, if another thread re-enables the gc while the current
thread is inside a critical section, things can break (the probability
is really
Changes by Charles-François Natali neolo...@free.fr:
--
keywords: +patch
Added file: http://bugs.python.org/file22476/heap_gc_deadlock.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12352
Changes by Charles-François Natali neolo...@free.fr:
Removed file: http://bugs.python.org/file22476/heap_gc_deadlock.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12352
___
Changes by Charles-François Natali neolo...@free.fr:
Added file: http://bugs.python.org/file22477/heap_gc_deadlock.diff
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue12352
___
STINNER Victor victor.stin...@haypocalc.com added the comment:
Or you can combine your two ideas:
in free(), perform a trylock of the mutex
if the trylock fails, then create a new Finalizer to postpone the
freeing of the same block to a later time,...
... perform the freeing of pending
Charles-François Natali neolo...@free.fr added the comment:
[...]
def free(self, block):
if self._lock.acquire(False):
self._exec_pending_free()
self._free(block)
else:
# malloc holds the lock
self._pending_free.append(block)
_pending_free uses a lock internally to
STINNER Victor victor.stin...@haypocalc.com added the comment:
_pending_free uses a lock internally to make it thread-safe, so I
think this will have exactly the same problem
You are probably right. Can't we use a lock-less list? list.append is
atomic thanks to the GIL, isn't it? But I don't
Charles-François Natali neolo...@free.fr added the comment:
The obvious solution is to use a recursive lock instead.
Note that it's not really a solution, just a workaround to avoid
deadlocks, become this might lead to a corruption if free is called
while the heap is in an inconsistent state.
greg.ath gathan...@gmail.com added the comment:
Hi,
I also wonder about the performance cost of a recursive lock.
I am still unable to reproduce the bug in a simple script. Looking
closely to the gdb stack, there is that frame:
Frame 0x13be190, for file
Charles-François Natali neolo...@free.fr added the comment:
Looking closely to the gdb stack, there is that frame:
Yeah, but it calls _free, which runs unlocked. That's not the problem.
I am still unable to reproduce the bug in a simple script.
Try with this one:
import
STINNER Victor victor.stin...@haypocalc.com added the comment:
I also wonder about the performance cost of a recursive lock.
An alternative is to disable the garbage collector in malloc():
def malloc(self, size):
...
enabled = gc.isenabled()
if enabled:
Charles-François Natali neolo...@free.fr added the comment:
Thanks for reporting this.
There's indeed a bug which can lead to this deadlock.
Relevant code in Lib/multiprocessing/heap.py
- the BufferWrapper class uses a single Heap() shared among instances,
protected by a mutex (threading.Lock),
New submission from greg.ath gathan...@gmail.com:
Hi,
My multithreaded application uses multithreading.Value() to ensure thread-safe
operations on shared data.
For unexpected reasons, after some change in my code, the function will
consistently hang.
I did a gdb backtrace of the hanging
34 matches
Mail list logo
