Skip to content

also redirect JL_STDERR etc. when redirecting to devnull #55958

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

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)

@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
@IanButterworth IanButterworth force-pushed the ib/redirect_stdio_devnull_c branch from f4ca02f to 9665f77 Compare October 2, 2024 19:35
@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?

This was referenced Oct 7, 2024
@KristofferC KristofferC mentioned this pull request Oct 18, 2024
43 tasks
@KristofferC KristofferC mentioned this pull request Oct 29, 2024
47 tasks
@KristofferC KristofferC mentioned this pull request Dec 3, 2024
51 tasks
This was referenced Jan 28, 2025
@KristofferC KristofferC mentioned this pull request Mar 11, 2025
71 tasks
@KristofferC KristofferC mentioned this pull request Mar 11, 2025
75 tasks
@KristofferC KristofferC mentioned this pull request Apr 25, 2025
71 tasks
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