unittest.mock.patch.object holds references for too long

[ppwwyyxx]

Bug report

Bug description:

import sys
from unittest import mock
import json

def main():
    obj = 'asdfa' * 2000

    print("BEGIN", sys.getrefcount(obj))
    for i in range(10):
        with mock.patch.object(json, 'dumps', return_value=3):
            from json import dumps
            dumps(obj)
    del dumps
    print("END", sys.getrefcount(obj))
    import gc; gc.collect()
    print("AFTERGC", sys.getrefcount(obj))


if __name__ == '__main__':
    main()

The code prints:

BEGIN 2
END 12
AFTERGC 2

Because it is the user's data that is leaked, the leaked object could be huge and could cause non-trivial problems.

CPython versions tested on:

3.13

Operating systems tested on:

Linux

Linked PRs

• gh-133423: Avoid ref cycle among MagicMock and its magic methods #139937

[picnixz]

obj still lives until the end of the function and lives as an argument to getrefcount so I don't think there is a leak.

[ppwwyyxx]

The code prints

BEGIN 2
END 12
AFTERGC 2

To be clear, it is the END 12 part that is unexpected.

[picnixz]

It's not unexpected. The interpreter may decide when to garbage collect something. It can decide to hold references longer than the user may expect.

However, we may want to optimize mocking in a way that we don't hold references for too long. I don't have access to my laptop now so I'll let @ZeroIntensity take over when possible (otherwise I'll do it myself)

[ZeroIntensity]

Yeah, some objects hold reference cycles, so the garbage collector has to do a pass before their references get freed. I don't think there's much we can do.

How is this causing an issue in practice? You can try adjusting the GC threshold if you want the garbage collector to run more frequently.

[ppwwyyxx]

I was expecting that we aim for fixing the reference cycles - or is this typically considered not worth fixing?

It is causing an issue in practice because the leaked object (argument of a mocked function) is holding precious resources (in my case, a large amount of GPU memory). If there were no ref cycle, the object would have been freed at the right time to make room for subsequent GPU memory allocations. With the issue present, subsequent GPU memory allocations would OOM.

[ZeroIntensity]

It is causing an issue in practice because the leaked object

Well, it's not leaking, it's just that it's being freed later than one would expect, because it needs a GC pass to free up the reference cycles. The object is still getting cleaned up at some point.

I think you should try adjusting the GC threshold, and if it's still an issue, we can possibly look into some early dels in unittest.mock (but I wouldn't be too hopeful!)

[picnixz]

I think the issue in this case is the use of a very large GPU resource in tests, and assuming that the environment has constrained memory. We might do something at the mock level but otherwise I don't think we have an actionable thing to do at the interpreter level.

For such tests I suggest either using a smaller dataset, shape and process the data differently or ensure that the machine that is executing the tests has sufficient memory.