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

Possible infinite recursion in PRINT_OR_APPEND_TO_FILE_OR_STREAM #3028

Closed
embray opened this issue Nov 19, 2018 · 6 comments
Closed

Possible infinite recursion in PRINT_OR_APPEND_TO_FILE_OR_STREAM #3028

embray opened this issue Nov 19, 2018 · 6 comments
Labels
kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) kind: bug Issues describing general bugs, and PRs fixing them

Comments

@embray
Copy link
Contributor

embray commented Nov 19, 2018

If a stream that is being used to print errors cannot be opened (e.g. an error elsewhere prevents streams from being closed properly) this can result in an infinite recursion when reporting that error.

The specific problem is in the error handling logic here:

i = OpenOutputStream(destination);

If an ErrorQuit (for example) occurs elsewhere, and the stream assigned to ERROR_OUTPUT cannot be opened for whatever reason, this results in another call to ErrorQuit with the same broken stream assigned and so on. Perhaps this should do something else: Try to open *errout* as a last resort, Panic(), etc.

@markuspf
Copy link
Member

See #2487

@embray
Copy link
Contributor Author

embray commented Nov 20, 2018

Yes, much of the discussion there is relevant here as well, although the exact issue here I think is not providing an appropriate fallback when we're trying to print error messages from within the error handler, and we can't open the specified stream.

@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) labels Mar 21, 2019
@fingolfin
Copy link
Member

I believe we resolved this some time ago (and the fixes are already in GAP 4.11). If you still can trigger an issue and/or believe there is one based on reading the code, please don't hesitate to reopen this and/or file a new issue.

@embray
Copy link
Contributor Author

embray commented Oct 13, 2020

I can believe it's been fixed but I'm not sure since the code I originally implicated hasn't been updated, but it might be dealt with elsewhere. I admit I don't recall how the issue was originally triggered. It might have shown up while developing the workaround I had to add for the Python libgap interface to capture error output: https://gitlab.com/sagemath/sage/-/blob/1e728ac0d36514f52871268588d38fe1392a5e5a/src/sage/libs/gap/util.pyx#L198

@fingolfin
Copy link
Member

There is a new dedicated function for opening errout in src/error.c with fallbacks in case there is an error doing so. See

UInt OpenErrorOutput( void )

@embray
Copy link
Contributor Author

embray commented Oct 13, 2020

I see, that definitely helps! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) kind: bug Issues describing general bugs, and PRs fixing them
Projects
None yet
Development

No branches or pull requests

3 participants