Confusing behavior with sys.monitoring.DISABLE

[sklam]

Bug report

Bug description:

# Adapted from https://github.com/python/cpython/blob/a5eeb832c2bbbd6ce1e9d545a553de926af468d5/Lib/test/test_monitoring.py#L670-L682
import sys



TEST_TOOL = 2
E = sys.monitoring.events

INSTRUMENTED_EVENTS = [
    (E.PY_START, "start"),
    # (E.PY_RETURN, "return"),  # Uncomment this line leads to different behavior
]


class CounterWithDisable:

    def __init__(self):
        self.disable = False
        self.count = 0

    def __call__(self, *args):
        print("cb", self)
        self.count += 1
        if self.disable:
            return sys.monitoring.DISABLE

def foo(x):
    return x + 1

def call_foo(x):
    yield 2 * foo(x + 5)



def test():
    sys.monitoring.use_tool_id(TEST_TOOL, "test")
    for event, name in INSTRUMENTED_EVENTS:
        print("Event", name)
        try:
            counter = CounterWithDisable()
            counter.disable = True
            sys.monitoring.register_callback(TEST_TOOL, event, counter)
            sys.monitoring.set_events(TEST_TOOL, event)
            list(call_foo(1))
            print("counter.count", counter.count)
            assert (counter.count < 4)
        finally:
            sys.monitoring.set_events(TEST_TOOL, 0)
            sys.monitoring.register_callback(TEST_TOOL, event, None)

    sys.monitoring.free_tool_id(TEST_TOOL)

print("First run".center(80, '='))
test()
print("Second run".center(80, '='))
test()

The above script prints:

===================================First run====================================
Event start
cb <__main__.CounterWithDisable object at 0x10944c0b0>
cb <__main__.CounterWithDisable object at 0x10944c0b0>
counter.count 2
===================================Second run===================================
Event start
counter.count 0

The first run "leaks" the disabled callback to the second run.

If I uncomment the line:

    # (E.PY_RETURN, "return"),  # Uncomment this line leads to different behavior

The script will print:

===================================First run====================================
Event start
cb <__main__.CounterWithDisable object at 0x10e34c1d0>
cb <__main__.CounterWithDisable object at 0x10e34c1d0>
counter.count 2
Event return
cb <__main__.CounterWithDisable object at 0x10e34c200>
cb <__main__.CounterWithDisable object at 0x10e34c200>
counter.count 2
===================================Second run===================================
Event start
cb <__main__.CounterWithDisable object at 0x10e34c2f0>
cb <__main__.CounterWithDisable object at 0x10e34c2f0>
counter.count 2
Event return
cb <__main__.CounterWithDisable object at 0x10e34c1d0>
cb <__main__.CounterWithDisable object at 0x10e34c1d0>
counter.count 2

It stops the "leaking".

This problem can be workarounded by calling sys.monitoring.restart_events() at the end of test(). But, it is not clear if it is necessary. This cpython test:

cpython/Lib/test/test_monitoring.py

Lines 670 to 682 in a5eeb83

	def test_disable_legal_events(self):
	for event, name in INSTRUMENTED_EVENTS:
	try:
	counter = CounterWithDisable()
	counter.disable = True
	sys.monitoring.register_callback(TEST_TOOL, event, counter)
	sys.monitoring.set_events(TEST_TOOL, event)
	for _ in self.gen(1):
	pass
	self.assertLess(counter.count, 4)
	finally:
	sys.monitoring.set_events(TEST_TOOL, 0)
	sys.monitoring.register_callback(TEST_TOOL, event, None)

does not use sys.monitoring.restart_events().

CPython versions tested on:

3.12

Operating systems tested on:

Linux, macOS

[gaogaotiantian]

I agree this is a bit confusing. We should probably define DISABLE better.

The de facto behavior of DISABLE now is:

• If restart_events() is called, all the events are guaranteed to be restored.
• If the events set on the code object changed, the DISABLE will be cleared.

In this specific case, the two times you called the call_foo function, the events set were the same. The code object was not aware of the fact set_events(0) happened, so it did not re-instrument the code, thus the DISABLE kept (not really leaked).

I think we could either make the documentation better (make sure under what circumstance the users have guarantee for disabled events), or we can come up with a behavior that's more consistent. @markshannon