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

also redirect JL_STDERR etc. when redirecting to devnull #55958

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

IanButterworth
Copy link
Member

Currently any c prints to JL_STDERR will print during redirect_stderr(f, devnull)

base/stream.jl Outdated Show resolved Hide resolved
@IanButterworth IanButterworth added backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 bugfix This change fixes an existing bug labels Oct 1, 2024
@IanButterworth

This comment was marked as outdated.

@IanButterworth

This comment was marked as outdated.

@IanButterworth IanButterworth force-pushed the ib/redirect_stdio_devnull_c branch 3 times, most recently from a3f3589 to 6d14d91 Compare October 2, 2024 15:31
@IanButterworth
Copy link
Member Author

@Keno can you take another look. I hadn't appreciated that it was _redirect_io_global(devnull not to handle at the end, so the deduplication needs to be more targetted.

@IanButterworth IanButterworth force-pushed the ib/redirect_stdio_devnull_c branch 2 times, most recently from 2ee5fbf to f4ca02f Compare October 2, 2024 18:16
@vtjnash
Copy link
Member

vtjnash commented Oct 2, 2024

This appears to be a use-after-free because of the close call, so I am not sure this will work

@IanButterworth
Copy link
Member Author

If I switch #55959 to use devnull, and add this change then I get the desired suppression without error, and CI seems happy here.

Screenshot 2024-10-02 at 8 56 43 PM

If the implementation isn't quite right, is there a better way? Use the handle after it's been dup-ed, somehow? Or just don't close it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 1.10 Change should be backported to the 1.10 release backport 1.11 Change should be backported to release-1.11 bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants