Title: [Windows] os.spawn*() tests of test_os leak references on Windows
Type: resource usage Stage:
Components: Tests, Windows Versions: Python 3.7
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: eryksun, haypo, paul.moore, steve.dower, tim.golden, zach.ware
Priority: normal Keywords:

Created on 2017-06-08 16:18 by haypo, last changed 2017-06-20 10:17 by haypo.

File name Uploaded Description Edit haypo, 2017-06-08 16:18
Pull Requests
URL Status Linked Edit
PR 2184 merged haypo, 2017-06-14 09:54
PR 2212 merged haypo, 2017-06-15 11:20
PR 2287 open haypo, 2017-06-20 10:17
Messages (8)
msg295451 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-08 16:18
The leak can be reproduced with the minimum attached

C:\haypo\python>PCbuild\amd64\python_d.exe -m test -R 3:3 test_os
Run tests sequentially
0:00:00 [1/1] test_os
beginning 6 repetitions
test_os leaked [2, 2, 2] memory blocks, sum=6
test_os failed

1 test failed:

Total duration: 172 ms
Tests result: FAILURE
msg295462 - (view) Author: Eryk Sun (eryksun) * Date: 2017-06-08 20:09
The memory leak is in os_spawnv_impl and os_spawnve_impl in Modules/posixmodule.c. The call fails with a ValueError when the first argument in the argv list is an empty string, in which case these functions both mistakenly pass i (0) to free_string_array() as the count of strings to free. But the conversion was successful, so it needs to include the current string in the count, i.e. `i + 1`.

The fix for this could also address the following two issues, which are mostly cosmetic in nature. os_spawnve_impl needs its TypeError message to be special cased the same as in os_spawnv_impl, i.e. "spawnve() arg 2 must contain only strings". Currently it uses the default message from the failed conversion: "expected str, bytes or os.PathLike object, not %.200s". Also, its ValueError message needs to reference "spawnve()" instead of "spawnv()".
msg295508 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-09 09:39
Eryk: do you want to work on a PR to fix it? It seems like you understand well the bug!
msg295999 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-14 12:26
New changeset 526b22657cb18fe79118c2ea68511aca09430c2c by Victor Stinner in branch 'master':
bpo-30602: Fix refleak in os.spawnve() (#2184)
msg296082 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-15 11:26
Oh, I fixed a leak in test_spawnve_noargs() of test_os, but test_spawnv_noargs() still leaks :-) os.spawnv() is very similar to os.spawnve(). fixes os.spawnv() as well.

I tested that "python -m test -R 3:3 test_os" doesn't leak anymore with this second fix.
msg296086 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-15 13:30
New changeset 8acb4cf2b3436652568d7a70228b166316181466 by Victor Stinner in branch 'master':
bpo-30602: Fix refleak in os.spawnv() (#2212)
msg296103 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-15 14:47
It seems like I introduced a bug:
msg296408 - (view) Author: STINNER Victor (haypo) * (Python committer) Date: 2017-06-20 10:12
The fixes must be backported to Python 3.6, test_os still leaks references on this branch:

test_os leaked [6, 6, 6] memory blocks, sum=18
Date User Action Args
2017-06-20 10:17:56hayposetpull_requests: + pull_request2335
2017-06-20 10:12:19hayposetmessages: + msg296408
2017-06-15 14:47:02hayposetmessages: + msg296103
2017-06-15 13:30:43hayposetmessages: + msg296086
2017-06-15 11:26:44hayposetmessages: + msg296082
2017-06-15 11:20:39hayposetpull_requests: + pull_request2256
2017-06-14 12:26:25hayposetmessages: + msg295999
2017-06-14 09:54:29hayposetpull_requests: + pull_request2234
2017-06-09 09:39:11hayposetmessages: + msg295508
2017-06-08 20:09:36eryksunsetnosy: + eryksun
messages: + msg295462
2017-06-08 16:18:47haypocreate