Re-implement IO::Handle getline and getlines as XS code

The bug reported twice about "Thread race in dist/IO/IO.xs" and now tracked
as https://github.com/Perl/perl5/issues/14816

relates to a regression introduced fixing a *different* bug reported as
https://github.com/perl/perl5/issues/11441
https://rt.cpan.org/Public/Bug/Display.html?id=66474

The underlying problem is that to implement getline and getlines correctly
execution needs to end up inside pp_readline(), but with the caller's
lexical state visible, not the method's.

Father C achieved this and fixed the bug by using a custom op checker to
install a custom op runtime which wrapped pp_nextstate, and (re)set the
interpreter context back to the callers after running the real nextstate,
and arranging to temporarily install *this* checker just while compiling
the two method bodies.

This works, but it turns out that the implementation (as-is) does not
modify the PL_check in a threadsafe way. Worse, the bug reporters realise
that this hides a deeper bug - PL_check itself should not need this level of
protection because it should be part of the interpreter's state, not global
state.

The ticket/issue has a patch to address this. I believe that it should fix
*that* bug and hence make IO threadsafe in blead. However, it will be hard
(to impossible) to backport to earlier perl versions.

In wondering about that, it occurred to me that we could address the visible
bug (loading the IO module being not threadsafe) by taking a different route
to Father C's end solution - running pp_readline with the caller's lexical
state - by re-implementing the two methods in XS.

This seems to work. Hence the branch smoke-me/nicholas/IO-Handle-getline_XS
to see what the smokers make of it. Tested on blead and a build of 5.8.1


But given how high in the "river" IO is, and how OS-dependent it can be,
and how many older versions of perl it needs to keep working on, I'd like
some specific help before pushing this to blead

1) Please could someone(s) check the XS code for sanity

2) Todd - are you able to make a developer release of IO to see what the
   cpantesters make of it, before we break CPAN

3) I extended the tests from the original bug report. Hence I have no great
   idea if they will work as-is on EBCDIC. Karl (or anyone) - are we able
   to test this


And in the context of smoke-me branches, and everything else:

4) The "migration to github" was described as being about bugs from RT to
   Github, with the potential of effectively swapping "master" and "mirror"
   between https://perl5.git.perl.org/perl.git and GitHub

   To my knowledge there's been nothing mentioned about how everything else
   is supposed to work. Historically we'd been using smoke-me branches to
   get automated testing. github "thinks" otherwise about how to describe
   something that would like testing, but the existing smokers don't know
   that.

   Likewise, historically all the requests I made here went to the list.
   In the brave new future, er, present, what *is* the plan for what goes
   where? We're now onto our third bug tracker, third hosting of version
   control, and not-sure-what iteration of mail archives.

   I feel that Github itself is in safer hands owned by Microsoft than in
   its previous state (gratuitously cashflow negative), but I'm wary of
   assuming that anything is going to be around forever, or is defaulting
   to what we really need.


Nicholas Clark
0
nick
11/6/2019 12:44:07 PM
perl.perl5.porters 47860 articles. 1 followers. Follow

5 Replies
8 Views

Similar Articles

[PageSpeed] 56

On 11/6/19 7:44 AM, Nicholas Clark wrote:
> The bug reported twice about "Thread race in dist/IO/IO.xs" and now tracked
> as https://github.com/Perl/perl5/issues/14816
> 
[snip]

Nicholas, would it be possible for you to separate your concerns 
expressed in item #4 below into a separate message?

Otherwise, we'll be discussing both the internals of IO:: and Perl 5 
infrastructure policy in the same thread.


> And in the context of smoke-me branches, and everything else:
> 
> 4) The "migration to github" was described as being about bugs from RT to
>     Github, with the potential of effectively swapping "master" and "mirror"
>     between https://perl5.git.perl.org/perl.git and GitHub
> 
>     To my knowledge there's been nothing mentioned about how everything else
>     is supposed to work. Historically we'd been using smoke-me branches to
>     get automated testing. github "thinks" otherwise about how to describe
>     something that would like testing, but the existing smokers don't know
>     that.
> 
>     Likewise, historically all the requests I made here went to the list.
>     In the brave new future, er, present, what *is* the plan for what goes
>     where? We're now onto our third bug tracker, third hosting of version
>     control, and not-sure-what iteration of mail archives.
> 
>     I feel that Github itself is in safer hands owned by Microsoft than in
>     its previous state (gratuitously cashflow negative), but I'm wary of
>     assuming that anything is going to be around forever, or is defaulting
>     to what we really need.
> 
> 
> Nicholas Clark
> 

Thank you very much.
Jim Keenan
0
jkeenan
11/6/2019 1:01:47 PM
On Wed, Nov 06, 2019 at 08:01:47AM -0500, James E Keenan wrote:
> On 11/6/19 7:44 AM, Nicholas Clark wrote:
> > The bug reported twice about "Thread race in dist/IO/IO.xs" and now tracked
> > as https://github.com/Perl/perl5/issues/14816
> > 
> [snip]
> 
> Nicholas, would it be possible for you to separate your concerns expressed
> in item #4 below into a separate message?
> 
> Otherwise, we'll be discussing both the internals of IO:: and Perl 5
> infrastructure policy in the same thread.

Really good point. Thanks.

That bit re-mailed with a new subject.
Please don't reply to this thread about the meta-level questions about
github, Travis, smoke-me and wanting it all.*

Nicholas Clark

* And a pony?
0
nick
11/6/2019 2:20:07 PM
On 11/6/19 5:44 AM, Nicholas Clark wrote:
> 3) I extended the tests from the original bug report. Hence I have no great
>     idea if they will work as-is on EBCDIC. Karl (or anyone) - are we able
>     to test this

Both the tests pass on blead with with this patch on z/OS.
0
public
11/7/2019 4:10:19 AM
On Wed, Nov 06, 2019 at 09:10:19PM -0700, Karl Williamson wrote:
> On 11/6/19 5:44 AM, Nicholas Clark wrote:
> > 3) I extended the tests from the original bug report. Hence I have no great
> >     idea if they will work as-is on EBCDIC. Karl (or anyone) - are we able
> >     to test this
> 
> Both the tests pass on blead with with this patch on z/OS.

Cool, thanks for testing.

Nicholas Clark
0
nick
11/7/2019 6:36:28 AM
On 11/6/19 11:36 PM, Nicholas Clark wrote:
> On Wed, Nov 06, 2019 at 09:10:19PM -0700, Karl Williamson wrote:
>> On 11/6/19 5:44 AM, Nicholas Clark wrote:
>>> 3) I extended the tests from the original bug report. Hence I have no great
>>>      idea if they will work as-is on EBCDIC. Karl (or anyone) - are we able
>>>      to test this
>>
>> Both the tests pass on blead with with this patch on z/OS.
> 
> Cool, thanks for testing.
> 
> Nicholas Clark
> 

I neglected to say that they passed on both threaded and unthreaded builds
0
public
11/7/2019 4:10:23 PM
Reply: