- Original Message -
From: Nir Soffer nsof...@redhat.com
To: Dan Kenigsberg dan...@redhat.com
Cc: Yaniv Bronheim ybron...@redhat.com, VDSM Project Development
vdsm-devel@lists.fedorahosted.org
Sent: Tuesday, February 4, 2014 4:39:55 PM
Subject: Re: [vdsm] suggested patch for python-pthreading
- Original Message -
From: Dan Kenigsberg dan...@redhat.com
To: Nir Soffer nsof...@redhat.com
Cc: Yaniv Bronheim ybron...@redhat.com, VDSM Project Development
vdsm-devel@lists.fedorahosted.org
Sent: Tuesday, February 4, 2014 3:51:02 PM
Subject: Re: [vdsm] suggested patch for python-pthreading
On Tue, Feb 04, 2014 at 05:14:51AM -0500, Nir Soffer wrote:
- Original Message -
From: Yaniv Bronheim ybron...@redhat.com
To: Nir Soffer nsof...@redhat.com
Cc: VDSM Project Development vdsm-devel@lists.fedorahosted.org
Sent: Tuesday, February 4, 2014 11:53:52 AM
Subject: Re: [vdsm] suggested patch for python-pthreading
- Original Message -
From: Nir Soffer nsof...@redhat.com
To: Yaniv Bronheim ybron...@redhat.com
Cc: VDSM Project Development vdsm-devel@lists.fedorahosted.org
Sent: Tuesday, February 4, 2014 11:39:56 AM
Subject: Re: [vdsm] suggested patch for python-pthreading
- Original Message -
From: Yaniv Bronheim ybron...@redhat.com
To: VDSM Project Development vdsm-devel@lists.fedorahosted.org
Sent: Tuesday, February 4, 2014 11:04:37 AM
Subject: [vdsm] suggested patch for python-pthreading
according to coredumps we found in the scope of the bug [1] we
opened
[2]
that suggested to override python's implementation of
thread.allocate_lock
in each coredump we saw few threads stuck with the bt:
#16 0x7fcb69288c93 in PyEval_CallObjectWithKeywords
(func=0x2527820,
arg=0x7fcb6972f050, kw=value optimized out) at
Python/ceval.c:3663
#17 0x7fcb692ba7ba in t_bootstrap (boot_raw=0x250a820) at
Modules/threadmodule.c:428
#18 0x7fcb68fa3851 in start_thread (arg=0x7fcb1bfff700) at
pthread_create.c:301
#19 0x7fcb6866694d in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:115
in pystack the threads were stuck in
/usr/lib64/python2.6/threading.py
(513): __bootstrap_inner
in bootstrap_inner we use thread.allocate_lock which
python-pthreading
does
not override.
we suggest the following commit:
From 9d89e9be1a379b3d93b23dd54a381b9ca0973ebc Mon Sep 17 00:00:00
2001
From: Yaniv Bronhaim ybron...@redhat.com
Date: Mon, 3 Feb 2014 19:24:30 +0200
Subject: [PATCH] Mocking thread.allocate_lock with Lock imp
Signed-off-by: Yaniv Bronhaim ybron...@redhat.com
---
pthreading.py | 4
1 file changed, 4 insertions(+)
diff --git a/pthreading.py b/pthreading.py
index 916ca7f..96df42c 100644
--- a/pthreading.py
+++ b/pthreading.py
@@ -132,6 +132,10 @@ def monkey_patch():
Thus, Queue and SocketServer can easily enjoy them.
+import thread
+
+thread.allocate_lock = Lock
+
import threading
threading.Condition = Condition
--
1.8.3.1
[1] https://bugzilla.redhat.com/show_bug.cgi?id=1022036
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1060749
Replacing allocate_lock in thread is correct. However, since
threading
copies
thread.allocate_lock, and you don't control import order, you have to
monkeypatch
threading.allocate_lock as well.
The full should be:
import thread
thread.allocate_lock = Lock
import threading
threading.allocate_lock = Lock
threading.Lock = Lock
Nir
thanks Nir for the reply and review
I guess you meant threading._allocate_lock, but we monkey patch it in
an
order so we do control the import order.. not sure its necessary
Correct, threading._allocate_lock. And we also have to patch
threading._active_limbo_lock.
We don't control the user import order - If I import threading before
pthreading, both
threading._allocate_lock and threading._active_limbo_lock are using the
incorrect lock.
Since many library modules import threading, it is impossible to require
any import order.
The safest way would be to patch all places that may use the original
thread.allocate_lock.
The safest way would be to say loud and clear: the user must call
pthreading.monkey_patch() on the begining of his main module, before
threading is imported and most certainly before it is ever used.
If we give the impression that doing anything else is fine, and we can
end up replacing _active_limbo_lock while
