Issue 44116: The _csv module can't be garbage-collected after _csv.register_dialect is called

After `_csv.register_dialect` is called, the csv module is alive even after it's removed from sys.modules. It should be garbage-collected.
(It's not that big a deal: unloading _csv isn't something users should do. But it might be hiding a deeper issue.)

The following reproducer (for a debug build of Python) shows an increasing number of refcounts. (Importing `csv` is the easiest way to call _csv._register_dialect with a proper argument):


import sys
import gc

for i in range(10):
    import csv
    del sys.modules['_csv']
    del sys.modules['csv']
    del csv
    gc.collect()

    print(sys.gettotalrefcount())

Hm, a similar thing apparently happens with urllib.request.

Could it be that the _csv heap types are not garbage collected? Ref. bpo-42972.

Adding GC to _csv types:
$ cat 
import sys
import gc

for i in range(10):
    import csv
    del sys.modules['_csv']
    del sys.modules['csv']
    del csv
    gc.collect()

    print(sys.gettotalrefcount())
$ ./python.exe bug.py
73164
73164
73166
73166
73166
73166
73166
73166
73166
73166

I created a quick-and-dirty patch. I can clean it up and make it into a PR if you want.

*facepalm* Yes, that's it.

If you have the time now, could you send he PR?

Sure, I’ll do it after dinner :)

The urllib.request one was caused by _hashlib, see GH-26072.

I don't understand this. After applying PR-26074, test_csv now leaks memory/refs:

$ ./python.exe -m test -R : test_csv
0:00:00 load avg: 1.18 Run tests sequentially
0:00:00 load avg: 1.18 [1/1] test_csv
beginning 9 repetitions
123456789
.........
test_csv leaked [3928, 3924, 3924, 3924] references, sum=15700
test_csv leaked [1666, 1664, 1664, 1664] memory blocks, sum=6658
test_csv failed

== Tests result: FAILURE ==

1 test failed:
    test_csv

Total duration: 4.9 sec
Tests result: FAILURE


Any ideas?

Also, for some reason two first iterations of the reproducer prints 2 less ref counts.

Changes to _csv.Error should not be necessary, there everything is handled by the superclass.

> Changes to _csv.Error should not be necessary, there everything is handled by the superclass.

Got it; thanks.

Yeah, that helped a lot: test_csv leaked [487, 487, 487, 487] memory blocks, sum=1948

Thanks! :)

New changeset ba260acb22f3a6de434dc7a159ddb94a6a8c9b7c by Miss Islington (bot) in branch '3.10':
bpo-44116: Add GC support to _csv heap types (GH-26074) (GH-26081)
https://github.com/python/cpython/commit/ba260acb22f3a6de434dc7a159ddb94a6a8c9b7c

Pablo, as mentioned in bpo-42972, this is an issue with all heap allocated types; it is not _csv specific. I know that work with heap types have been halted by the SC, as you've pointed out a couple of times already, but shouldn't this heap type issue be fixed before 3.10 is released? There's plenty of time in the beta phase to do this.

> I know that work with heap types have been halted by the SC, as you've pointed out a couple of times already, but shouldn't this heap type issue be fixed before 3.10 is released?

It should absolutely be fixed for all types that have been already converted. The halting is for more types.

pull_requests: + pull_request24720
stage: patch review