Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Dutcho mannequin opened this issue Jan 1, 2020 · 11 comments
Open

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

Dutcho mannequin opened this issue Jan 1, 2020 · 11 comments
Labels
3.8 (EOL) end of life 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@Dutcho
Copy link
Mannequin

Dutcho mannequin commented Jan 1, 2020

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']

@Dutcho
Copy link
Mannequin Author

Dutcho mannequin commented Jan 1, 2020

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. ...

@Dutcho Dutcho mannequin added 3.8 (EOL) end of life type-bug An unexpected behavior, bug, or error labels Jan 1, 2020
@zooba
Copy link
Member

zooba commented Jan 3, 2020

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 zooba added 3.9 only security fixes labels Jan 3, 2020
@terryjreedy
Copy link
Member

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
Copy link
Member

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.

@zooba
Copy link
Member

zooba commented Jan 4, 2020

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.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
@iritkatriel iritkatriel added the interpreter-core (Objects, Python, Grammar, and Parser dirs) label Nov 26, 2023
@devdanzin
Copy link
Contributor

devdanzin commented Oct 22, 2024

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.

@zooba
Copy link
Member

zooba commented Oct 23, 2024

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).

@devdanzin
Copy link
Contributor

devdanzin commented Oct 23, 2024

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?

@zooba
Copy link
Member

zooba commented Oct 23, 2024

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.

@devdanzin
Copy link
Contributor

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

@zooba
Copy link
Member

zooba commented Oct 23, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.8 (EOL) end of life 3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

4 participants