cpython icon indicating copy to clipboard operation
cpython copied to clipboard

sys.addaudithook(hook) loops indefinitely on mismatch for hook

Open 11571bd7-cc8f-4b80-a34f-d13719c58596 opened this issue 6 years ago • 11 comments

BPO 39182
Nosy @terryjreedy, @tiran, @zooba, @Dutcho

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2020-01-01.17:23:00.844>
labels = ['type-bug', '3.8', '3.9']
title = 'sys.addaudithook(hook) loops indefinitely on mismatch for hook'
updated_at = <Date 2020-01-04.19:40:23.157>
user = 'https://github.com/Dutcho'

bugs.python.org fields:

activity = <Date 2020-01-04.19:40:23.157>
actor = 'steve.dower'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2020-01-01.17:23:00.844>
creator = 'Dutcho'
dependencies = []
files = []
hgrepos = []
issue_num = 39182
keywords = []
message_count = 5.0
messages = ['359164', '359247', '359279', '359280', '359307']
nosy_count = 4.0
nosy_names = ['terry.reedy', 'christian.heimes', 'steve.dower', 'Dutcho']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue39182'
versions = ['Python 3.8', 'Python 3.9']

When hook is not a compatible callable, addaudithook() will loop forever. At the minimum, a check for being callable should be executed. Preferably, a non-compatible (i.e. signature != [[str, tuple], Any]) hook callable should also be detected.

>py
Python 3.8.1 (tags/v3.8.1:1b293b6, Dec 18 2019, 23:11:46) [MSC v.1916 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.addaudithook(0)
error=10
Exception ignored in audit hook:
TypeError: 'int' object is not callable
  File "<stdin>", line 0
SyntaxError: unknown parsing error
error=10
Exception ignored in audit hook:
TypeError: 'int' object is not callable
  File "<stdin>", line 0
SyntaxError: unknown parsing error
error=10
Exception ignored in audit hook:
TypeError: 'int' object is not callable
  File "<stdin>", line 0
SyntaxError: unknown parsing error
... etc. ...

I think this is specific to the interactive prompt. Under normal circumstances, any error in calling the hook at the point where an event is being audited (which is immediate when using interactive mode) needs to propagate.

Probably the best way to handle it is to call the hook with a new event indicating it's being added, but before it actually gets put into the list. A hook could then set up a different kind of loop, but it would be much harder to do it by accident.

Perhaps "sys.addaudithook/self" with itself as an argument. We can't just use "sys.addaudithook" in case there are hooks that blindly abort this event (which there are, because I've written them ;) ). But this will at least make sure that the signature is correct and shouldn't send the interactive parser into a loop.

zooba avatar Jan 03 '20 18:01 zooba

I think this is specific to the interactive prompt. In IDLE, which simulates interactive mode with repeated 'exec(user_code, namespace)', no error is printed, let alone a loop.

>> import sys >> sys.addaudithook(0) >>

terryjreedy avatar Jan 04 '20 06:01 terryjreedy

I wrote too soon. Entering anything at the next line prints TypeError: 'int' object is not callable twice and a lot of IDLE specific stuff, and not another prompt. I have to restart the remote execution process.

terryjreedy avatar Jan 04 '20 06:01 terryjreedy

Right, IDLE doesn't call exec/compile until _after_ the code is submitted, while the regular prompt calls it first and passes stdin as the source file (which then does a buffered read).

The loop occurs because the next action after an invalid input is to read another input, which is correct, it just doesn't (and cannot) detect the case where an error is raised _before_ the user presses Enter.

zooba avatar Jan 04 '20 19:01 zooba

As reported in # #125852 (which is a dupe of this), this issue causes PyREPL to exit both in Windows and Linux.

Should I submit a simple PR adding a if (!PyCallable_Check(hook)) check? If so, what should happen if it isn't a callable: just return None or raise an exception?

Edit: that won't work, as a callable can become non-callable after the check was done. But maybe checking at the time of calling the hook can work.

devdanzin avatar Oct 22 '24 19:10 devdanzin

Or as I suggested, call the hook before it gets added to the list. If it's not callable, you get an error immediately. If it changes behaviour later, well, that's what the "we're all consenting adults here" rule is for (i.e. don't change behaviour later unless you want to break yourself).

zooba avatar Oct 23 '24 15:10 zooba

Or as I suggested, call the hook before it gets added to the list.

Currently, a hook raising an exception fails silently (and stops calling of hooks added later), right? If so, would that behavior change (don't add the hook if it raises any exception) or would we only skip adding it if the exception indicates it's not callable?

Wouldn't it be simpler to check whether it's callable at adding time, if we don't care whether it's always callable?

devdanzin avatar Oct 23 '24 15:10 devdanzin

Wouldn't it be simpler to check whether it's callable at adding time, if we don't care whether it's always callable?

That's what I said. Make addaudithook call the hook with some event that means "you're about to be added" and don't handle any exceptions.

zooba avatar Oct 23 '24 17:10 zooba

Sorry, I meant checking if (!PyCallable_Check(hook)) instead, to avoid any side effects from calling the hook.

devdanzin avatar Oct 23 '24 17:10 devdanzin

A hook that has side effects for an unknown event is already in a decent amount of trouble, but finding that out immediately (especially if it's an error) is much more helpful, and will only be found by actually calling it.

zooba avatar Oct 23 '24 20:10 zooba