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

retain the last file handle even if it's an IO reference #19125

Draft
wants to merge 1 commit into
base: blead
Choose a base branch
from

Conversation

tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Sep 14, 2021

It's always been possible to extract a reference to the IO SV from
a glob, and use that for I/O, but such use would set PL_last_in_gv
to a temporary GV which would almost immediately be released,
clearing PL_last_in_gv.

This would result in reads from $. returning its most recently read
value (even from another handle) and modification being ignored, while
${^LAST_FH} would return undef, even though the file handle is still
open.

To fix this, store the underlying IO object as well as the GV.
Retaining the GV lets errors continue to report the GV name where
possible, while the IO object means $. and ${^LAST_FH} can still work
if the IO object still exists.

This does not try to fix the problem where localization can leave
PL_last_in_gv (and now PL_last_in_io) pointing at a SV that has been
destroyed, the original changes that introduced PL_last_in_gv didn't
keep a reference count for the GV, since this would prevent the file
being close automatically when the reference to to the glob went out
of scope.

Fixes #1420

@Tux
Copy link
Contributor

Tux commented Mar 7, 2022

My gut feeling tells me this will either fix a lot of unexplored format/write problems too.
OTOH it might also cause some new :)

Anyway this is beyond my area of expertise to give a +1 or -1 on the code as shown.

It's always been possible to extract a reference to the IO SV from
a glob, and use that for I/O, but such use would set PL_last_in_gv
to a temporary GV which would almost immediately be released,
clearing PL_last_in_gv.

This would result in reads from $. returning its most recently read
value (even from another handle) and modification being ignored, while
${^LAST_FH} would return undef, even though the file handle is still
open.

To fix this, store the underlying IO object as well as the GV.
Retaining the GV lets errors continue to report the GV name where
possible, while the IO object means $. and ${^LAST_FH} can still work
if the IO object still exists.

This does not try to fix the problem where localization can leave
PL_last_in_gv (and now PL_last_in_io) pointing at a SV that has been
destroyed, the original changes that introduced PL_last_in_gv didn't
keep a reference count for the GV, since this would prevent the file
being close automatically when the reference to to the glob went out
of scope.

Fixes Perl#1420
@demerphq
Copy link
Collaborator

demerphq commented Feb 8, 2023

@tonycoz this seems reasonable, but stalled and in conflict now. Do you want to pick it up? Should I try to resolve conflicts on it?

@tonycoz
Copy link
Contributor Author

tonycoz commented Feb 8, 2023

I think I left it draft (it's been a while) because of that last problem - IIRC it was PL_last_in_gv wasn't being reference counted when pushed to the save stack, so we could be restoring a freed GV to PL_last_in_gv when the save stack was popped.

I suspect the fix is to increment the reference count when saving, and decrement when restoring, setting PL_last_in_gv to NULL if the reference count is zero.

That could result in a user visible change of behaviour though, and might need a custom save type.

Feel free to pick it up.

@tonycoz
Copy link
Contributor Author

tonycoz commented May 25, 2023

also fixes #17655

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

*FH{IO} and $. scoping ickiness
3 participants