[3.12] gh-127637: add tests for dis command-line interface (#127759)

[picnixz]

(cherry picked from commit e85f2f1)

• Issue: Add tests for dis command-line interface #127637

[picnixz]

Apparently, test_embed get stucked. I won't have time for more work until Friday (and I can't test it since I don't have a Windows testing machine).

[iritkatriel]

Apparently, test_embed get stucked. I won't have time for more work until Friday (and I can't test it since I don't have a Windows testing machine).

I doubt it's related to this PR.

[picnixz]

@iritkatriel Do you want to merge this as is?

[iritkatriel]

@iritkatriel Do you want to merge this as is?

I can't, the button is greyed out when there are failing tests.

[picnixz]

Ok, so the tests consistently fail. Other backports PRs don't have this issue. I'll disable the automerge and check if removing the test actually changes something.

[picnixz]

What I found is that test_specialized_static_code_gets_unspecialized_at_Py_FINALIZE imports dis. I'm not sure if there is something happening because of this (maybe sys.argv is incorrectly picked up? or something else? I don't know)

[picnixz]

OK so now the tests passed. I don't know why. Is it because of tempfile perhaps?

[picnixz]

Ok, so after some investigation it appears that the import test.test_dis was the bad line. I don't really know why but I assume it has to be because of some import magic. Failures related to WMI should however be unrelated.

[picnixz]

@iritkatriel Can you perhaps explain to me why removing the import of test.test_dis made the test work? is it because of the main() function of dis that has been changed? is it because of the tempfile import? or is it because I'm importing opcode before test* and others?

[iritkatriel]

@iritkatriel Can you perhaps explain to me why removing the import of test.test_dis made the test work? is it because of the main() function of dis that has been changed? is it because of the tempfile import? or is it because I'm importing opcode before test* and others?

I don't know. Could you squash the commits so that the diff between the working and non working versions is clear, and then we can ask an import expert?

[picnixz]

Ok, now here's the commit that fixed everything: 68a33f3

[iritkatriel]

Ok, now here's the commit that fixed everything: 68a33f3

ok, now revert this commit so we see the error, but leave this commit and the revert commit in the log.

[picnixz]

ok, now revert this commit so we see the error,

AFAIR, the error was a timeout (after 20 minutes) and a simple "FAIL". But the stacktrace could help the import expert (AFAICT, the execution appears to wait for some input, and I thought it was because it was trying to run dis.main() itself but maybe it's something else)

[iritkatriel]

ok, now revert this commit so we see the error,

AFAIR, the error was a timeout (after 20 minutes) and a simple "FAIL". But the stacktrace could help the import expert (AFAICT, the execution appears to wait for some input, and I thought it was because it was trying to run dis.main() itself but maybe it's something else)

It didn't happen on 3.13, right?

[picnixz]

It didn't happen on 3.13, right?

No, the 3.13 backport went well.

[iritkatriel]

It didn't happen on 3.13, right?

No, the 3.13 backport went well.

Let's see if the release manager for 3.12 has any ideas then. @Yhg1s

[Yhg1s]

I think the bug this exposes is real, but not caused by the change here. It may just be a bug in test_embed's test program (Programs/_testembed.c). I managed to reproduce it locally (on Windows). The timeout is because the _testembed program crashes, and Windows (in debug builds) produces a popup dialog asking if you want to abort. It is a real abort, an assertion failure:

PS D:\Python\cpython> D:\\Python\\cpython\\PCbuild\\amd64\\_testembed_d.exe test_repeated_init_exec 'import dis
>> import importlib._bootstrap
>> import opcode
>> import test.test_dis
>>
>> def is_specialized(f):
>>     for instruction in dis.get_instructions(f, adaptive=True):
>>         opname = instruction.opname
>>         if (
>>             opname in opcode._specialized_instructions
>>             # Exclude superinstructions:
>>             and \"__\" not in opname
>>         ):
>>             return True
>>     return False
>>
>> func = importlib._bootstrap._handle_fromlist
>>
>> # "copy" the code to un-specialize it:
>> func.__code__ = func.__code__.replace()
>>
>> assert not is_specialized(func), \"specialized instructions found\"
>>
>> for i in range(test.test_dis.ADAPTIVE_WARMUP_DELAY):
>>     func(importlib._bootstrap, [\"x\"], lambda *args: None)
>>
>> assert is_specialized(func), \"no specialized instructions found\"
>>
>> print(\"Tests passed\")'
--- Loop #1 ---
Tests passed
--- Loop #2 ---
Assertion failed: PyUnicode_CheckExact(ep->me_key), file D:\Python\cpython\Objects\dictobject.c, line 939

As you can see it happens on the second time running the test code (after finalizing and reinitializing), and the ep the assertion fails on is a deallocated object. I think the problem is something isn't refcounted correctly, isn't marked as immortal when it should be, or isn't recreated on interpreter starutp when it should be. I don't know why it doesn't reproduce on Linux in debug mode.

I'm not comfortable enough with the Windows debugging environment to dig into this more. I'm pretty sure this is only a problem with repeated Py_Finalize/Py_Initialize calls. If either of you want to start an educational deep-dive into object reclamation/recreation during interpreter finalization and initialization, feel free, but since this only happens in 3.12, and lots has changed in 3.13 already around object lifetimes, I'm happy with the workaround you have as well. (I think the test is neater if it doesn't try to import test_dis anyway.)

[picnixz]

@vstinner I plan to merge this one with no commit message and the PR as the title.

[vstinner]

LGTM