classification
Title: Removing thread reference in thread results in leaked reference
Type: Stage: resolved
Components: Versions: Python 3.10
process
Status: closed Resolution: duplicate
Dependencies: Superseder: fix for bpo-36402 (threading._shutdown() race condition) causes reference leak
View: 37788
Assigned To: Nosy List: jaraco, martin.panter, ronaldoussoren
Priority: normal Keywords:

Created on 2020-11-04 19:42 by jaraco, last changed 2020-11-05 01:28 by jaraco. This issue is now closed.

Messages (5)
msg380353 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2020-11-04 19:42
In issue37193, I'd worked on an implementation in which a thread reference would be removed as the thread was closing, but this led to a memory leak caught by the buildbots (https://bugs.python.org/issue37193#msg380172).

As I tracked down the issue in GH-23127, I discovered that removing the reference to the thread from within the thread triggered the reference leak detection.

I've distilled that behavior into its own test which fails on master:

```
cpython master $ git diff
diff --git a/Lib/test/test_threading.py b/Lib/test/test_threading.py
index e0e5406ac2..2b4924d4d0 100644
--- a/Lib/test/test_threading.py
+++ b/Lib/test/test_threading.py
@@ -443,6 +443,15 @@ def _run(self, other_ref, yet_another):
                          msg=('%d references still around' %
                               sys.getrefcount(weak_raising_cyclic_object())))
 
+    def test_remove_thread_ref_in_thread(self):
+        def remove_ref():
+            threads[:] = []
+
+        threads = [
+            threading.Thread(target=remove_ref),
+        ]
+        threads[0].start()
+
     def test_old_threading_api(self):
         # Just a quick sanity check to make sure the old method names are
         # still present
```

Running that test with refcount checks leads to this failure:

```
cpython master $ ./python.exe Tools/scripts/run_tests.py -R 3:3 test.test_threading -m test_remove_thread_ref_in_thread
/Users/jaraco/code/public/cpython/python.exe -u -W default -bb -E -m test -r -w -j 0 -u all,-largefile,-audio,-gui -R 3:3 test.test_threading -m test_remove_thread_ref_in_thread
Using random seed 8650903
0:00:00 load avg: 1.76 Run tests in parallel using 10 child processes
0:00:01 load avg: 1.78 [1/1/1] test.test_threading failed

== Tests result: FAILURE ==

1 test failed:
    test.test_threading
0:00:01 load avg: 1.78
0:00:01 load avg: 1.78 Re-running failed tests in verbose mode
0:00:01 load avg: 1.78 Re-running test.test_threading in verbose mode
beginning 6 repetitions
123456
......
test.test_threading leaked [1, 1, 1] references, sum=3
test.test_threading leaked [3, 1, 1] memory blocks, sum=5
beginning 6 repetitions
123456
test_remove_thread_ref_in_thread (test.test_threading.ThreadTests) ... ok

----------------------------------------------------------------------

Ran 1 test in 0.001s

OK
.test_remove_thread_ref_in_thread (test.test_threading.ThreadTests) ... ok

----------------------------------------------------------------------

Ran 1 test in 0.001s

OK
.test_remove_thread_ref_in_thread (test.test_threading.ThreadTests) ... ok

----------------------------------------------------------------------

Ran 1 test in 0.001s

OK
.test_remove_thread_ref_in_thread (test.test_threading.ThreadTests) ... ok

----------------------------------------------------------------------

Ran 1 test in 0.001s

OK
.test_remove_thread_ref_in_thread (test.test_threading.ThreadTests) ... ok

----------------------------------------------------------------------

Ran 1 test in 0.001s

OK
.test_remove_thread_ref_in_thread (test.test_threading.ThreadTests) ... ok

----------------------------------------------------------------------

Ran 1 test in 0.001s

OK
.
test.test_threading leaked [1, 1, 1] references, sum=3
test.test_threading leaked [1, 1, 1] memory blocks, sum=3
1 test failed again:
    test.test_threading

== Tests result: FAILURE then FAILURE ==

1 test failed:
    test.test_threading

1 re-run test:
    test.test_threading

Total duration: 2.0 sec
Tests result: FAILURE then FAILURE
```

Is that behavior by design? Is it simply not possible to remove a reference to a thread from within the thread?
msg380357 - (view) Author: Ronald Oussoren (ronaldoussoren) * (Python committer) Date: 2020-11-04 20:19
Could this be a race condition?  The thread that's created in the test is not waited on (join), it may or may not have exited by the time the test function returns.
msg380381 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2020-11-04 23:34
I don't think it's a race condition for two reasons: adding a `time.sleep(1)` after `.start` still raises errors, and in issue37193, there were 10 threads created, with at least 9 of those reaching termination before the test ended, yet it showed 10 reference leaks.

If you join the thread in the test, the leak is not detected. However, I believe that's because, in order to join on the thread, you must also hold a handle to the thread, so the condition isn't triggered.
msg380388 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2020-11-05 01:05
Maybe this is related to (or duplicate of) Issue 37788? Python 3.7 has a regression where threads that are never joined cause leaks; previous code was written assuming you didn't need to join threads.

Do you still see the leak even if you don't clear the "threads" list (change the target to a no-op), or if you never create a list in the first place?
msg380390 - (view) Author: Jason R. Coombs (jaraco) * (Python committer) Date: 2020-11-05 01:28
Yes, I agree it's a duplicate of issue37788. 

And yes, it does still leak if the list is never created or if the target is a no-op.
History
Date User Action Args
2020-11-05 01:28:49jaracosetstatus: open -> closed
superseder: fix for bpo-36402 (threading._shutdown() race condition) causes reference leak
messages: + msg380390

resolution: duplicate
stage: resolved
2020-11-05 01:05:18martin.pantersetmessages: + msg380388
2020-11-05 00:53:37martin.pantersetnosy: + martin.panter
2020-11-04 23:34:10jaracosetmessages: + msg380381
2020-11-04 20:19:56ronaldoussorensetnosy: + ronaldoussoren
messages: + msg380357
2020-11-04 19:42:45jaracocreate