python 3.10 issue with RLock patcher existing locks

[arthurzam]

As reported at #715 (review)

Bad news: somewhere between 3.10.0b1 and 3.10.0rc1, test_patcher_existing_locks_locked started failing with RuntimeError: cannot release un-acquired lock around

eventlet/tests/isolated/patcher_existing_locks_locked.py

Line 19 in c6c350e

lock.release()

I'm still scratching my head over that one. It looks like gc.get_referrers(old) is mostly returning lists and dicts, though? I wonder if it's somehow related to the trouble I was seeing with __builtins__ showing up as a dict in socket_test.test_error_is_timeout...

I decided to bisect the CPython for the commit which broke this test. I have found this commit python/cpython@e30fe27 . When running the test suite, it passes on its parent commit, but fails on this commit (the same way it fails for released 3.10.0). Just as information, it was between 3.10.0_beta2 and 3.10.0_beta3.

---

Now, about how to fix it? Sadly I'm quite new to eventlet and I don't understand quite well the code here:

eventlet/eventlet/patcher.py

Lines 392 to 403 in 8904a33

	lock_type = type(threading.Lock())
	rlock_type = type(threading.RLock())
	if hasattr(threading, '_PyRLock'):
	# this happens on CPython3 and PyPy >= 7.0.0: "py3-style" rlocks, they
	# are implemented natively in C and RPython respectively
	py3_style = True
	pyrlock_type = type(threading._PyRLock())
	else:
	# this happens on CPython2.7 and PyPy < 7.0.0: "py2-style" rlocks,
	# they are implemented in pure-python
	py3_style = False
	pyrlock_type = None

But when I tested the types on multiple targets (py3.8, py3.9, py3.10, pypy7.3.5, pypy7.3.6) it always returned _thread.RLock for type(threading.RLock()).
So just as a test, I tried to manually set pyrlock_type = rlock_type at line 398, and the specific test passed. Here is the diff.

diff --git a/eventlet/patcher.py b/eventlet/patcher.py
index b249d6f..3f828a7 100644
--- a/eventlet/patcher.py
+++ b/eventlet/patcher.py
@@ -395,7 +395,7 @@ def _green_existing_locks():
         # this happens on CPython3 and PyPy >= 7.0.0: "py3-style" rlocks, they
         # are implemented natively in C and RPython respectively
         py3_style = True
-        pyrlock_type = type(threading._PyRLock())
+        pyrlock_type = rlock_type
     else:
         # this happens on CPython2.7 and PyPy < 7.0.0: "py2-style" rlocks,
         # they are implemented in pure-python

---

I will repeat that I have no idea if my change is even mildly correct. I just know it managed to make this test, and added no new regressions when all other tests were run. That is also the reason this isn't an Pull Request, and just an issue report.

My test environment is Gentoo, using master version of eventlet. Tested on CPython 3.8.12 and 3.9.7 (passes with and without the diff) and CPython 3.10.0 (needed the diff, but there were still other failures in other parts, even without the diff), and PyPy3-7.3.5 and PyPy3-7.3.6 (passes with and without the diff).

[temoto]

@arthurzam thank you very much.

I think with that patch, following code would not touch any locks because it would expect isinstance(obj, rlock_type) to be both True and False. So test probably doesn't verify strong enough that required fixes were applied.

        if isinstance(obj, rlock_type):
            elif py3_style and not isinstance(obj, pyrlock_type):
                _fix_py3_rlock(obj)

[arthurzam]

@temoto Oh, I see my mistake - instead of more "lock patching", I'm doing none. This means that the issue isn't in the place I reported, but in _fix_py3_rlock:

eventlet/eventlet/patcher.py

Lines 426 to 444 in 8904a33

	def _fix_py3_rlock(old):
	import gc
	import threading
	new = threading._PyRLock()
	while old._is_owned():
	old.release()
	new.acquire()
	if old._is_owned():
	new.acquire()
	gc.collect()
	for ref in gc.get_referrers(old):
	try:
	ref_vars = vars(ref)
	except TypeError:
	pass
	else:
	for k, v in ref_vars.items():
	if v == old:
	setattr(ref, k, new)

Which is quite more logical now that I think about it, as the CPython patch that "broke" us has changed traversing. So my new guess is we have an issue with our for ref in gc.get_referrers(old).

Back to investigation...

[tipabu]

I was looking at all this again recently, and rediscovered #546 (i.e., _green_existing_locks doesn't work on py3 anyway)

I think py310 finally got the existing RLocks to show up in gc.get_objects(), which is exposing how busted the never-before-exercised _fix_py3_rlock is...

[itamarst]

This was recently fixed.