bpo-30599: Fix a refleak in test_threaded_import

[vstinner]

Fix a reference leaks in test_threaded_import.

Replace "import random" with "import types" to avoid the following call of random.py, since "atfork" callbacks cannot be unregistered:

os.register_at_fork(_inst.seed, when='child')

Check if types.ModuleType attribute is available, rather than trying to call random.randrange().

[vstinner]

@pitrou: It seems like you wrote some of test_threaded_import tests, especially test_side_effect_import(): commit ea3eb88, http://bugs.python.org/issue9260.

Do you think that testing the availability of the types.ModuleType attribute is enough to check for non-regression?

[pitrou]

I don't think test_side_effect_import would be a problem, it only does a regular import.

[pitrou]

Is it something that actually happens?

[vstinner]

I can only say that the comment was written 16 years ago by @tim-one, commit aa22223... The import machinery was deeply modified since this commit was made... I'm not sure that the test still makes sense :-p

[serhiy-storchaka]

Could you use less common module (e.g. colorsys)? Unloading a module that is used by say unittest or libregrtest can cause problems. In particular types is directly imported by modulefinder, so importing it after importing modulefinder doesn't have any effect.

[pitrou]

Or, better, write a custom module that does a simplified version of what random does at import...

[vstinner]

Or, better, write a custom module that does a simplified version of what random does at import...

Since I don't undertand why was the import problem, I don't see how to write a module which would reproduce the bug.

[vstinner]

Serhiy: "Could you use less common module (e.g. colorsys)?"

Ok, done. I like colorsys since it has no dependency and its code is non-trivial. So we can keep a test trying to call a function (colorsys.rgb_to_hls()) of an imported module, rather than just testing if an attribute is available (types.ModuleType).

Does it look better now?

[pitrou]

I like colorsys since it has no dependency and its code is non-trivial.

The problem is this isn't exercising anything anymore.

[pitrou]

Since I don't undertand why was the import problem, I don't see how to write a module which would reproduce the bug.

Typically import a bunch of modules at top-level (just copy the imports done at the top of random.py), then create a singleton instance of something and bind one of its methods at the top-level, like random.py does. Then in the test, check you can call the top-level method.

[vstinner]

Typically import a bunch of modules at top-level (just copy the imports done at the top of random.py), then create a singleton instance of something and bind one of its methods at the top-level, like random.py does. Then in the test, check you can call the top-level method.

Hum, instead of cloning random.py "just to remove the os.register_at_fork() call", I chose a different approach which I already mention: #2029 only mocks os.register_at_fork() and so is less risky.

[vstinner]

Superseded by the PR #2029 which has been merged.