Repository navigation
Confusing behavior with sys.monitoring.DISABLE #118327
Copy link
Copy link
Open
Labels
type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Description
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
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
Activity
gaogaotiantian commentedon Apr 26, 2024
I agree this is a bit confusing. We should probably define
DISABLEbetter.The de facto behavior of
DISABLEnow is:restart_events()is called, all the events are guaranteed to be restored.DISABLEwill be cleared.In this specific case, the two times you called the
call_foofunction, the events set were the same. The code object was not aware of the factset_events(0)happened, so it did not re-instrument the code, thus theDISABLEkept (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