develooper Front page | perl.perl5.porters | Postings from June 2015

Re: 5.22 breaks Coro-6.42

Thread Previous | Thread Next
From:
Max Maischein
Date:
June 24, 2015 17:15
Subject:
Re: 5.22 breaks Coro-6.42
Message ID:
558AE5AA.6080404@corion.net
Hello Dave,

Am 24.06.2015 um 13:59 schrieb Dave Mitchell:
> On Mon, Jun 08, 2015 at 08:46:34PM +0200, Max Maischein wrote:
>> Hello Daniel,
>>
>> thank you for the description which I think should enable me to make the
>> changes to Coro.
>>
>> Am 31.05.2015 um 22:13 schrieb bulk88:
>>> Max Maischein wrote:
>>>> I did some spelunking in git history and some comparing between 5.20
>>>> and 5.22-RC2, and here are my findings (as on irc):
>>>>
>>>> There are two errors. One can be easily fixed by reverting the patch
>>>> c910fead7893fe9700031ee59de6b904260b5d69 :
>>>>
>>>> State.xs:3472:9: error: assignment of member 'svt_get' in read-only
>>>> object
>>>>          orig_sigelem_get = PL_vtbl_sigelem.svt_get;
>>>> PL_vtbl_sigelem.svt_get
>>>> = coro_sigelem_get;
>>>>          ^
>>>> [...]
>>>
>>> Since I wrote that patch the solution is make your own magic vtable
>>> (struct mgvtbl), copy the function pointers you want from the core's
>>> vtable RO vtable to your RW vtable in your XS module. Then call
>>> mg_findext to get the core created MG *, and assign your vtable to
>>> mg->mg_virtual replacing the core one.
>>
>> I think this approach is problematic since it will only replace the existing
>> magic for elements in %SIG that exist at patching time. Elements added to
>> %SIG after Coro has loaded will get the original Perl sigelem magic and not
>> the changed magic, at least that is how I interpret the C code and data. As
>> the main purpose of this vtable switching is to provide custom setters and
>> getters for $SIG{__DIE__} and $SIG{__WARN__}, I think there needs to be more
>> magic added to %SIG such that it attaches the correct kind of magic to
>> elements stored in
>> it.
>>
>> Is the above interpretation correct or am I overlooking something?
>
>
> That sounds about right.
>
> You have to construct a new vtable for %SIG, then set mg_virtual on %SIG
> to point to it (it's currently null), and set the MGf_COPY flag.  In the
> vtable, add a svt_copy method, which will be responsible for attaching
> magic to new $SIG elements. (see Perl_mg_copy). If the element is __WARN__
> or __DIE__, make the element point to a new vtable.
>
> Also at BOOT: time, check %SIG for existing __WARN__ and __DIE__ entries,
> and if they exist, change their mg_virtual pointer.
>
Yes, I have corresponding code as an interim patch for that already, but 
not yet sent it to Marc. The long-term fix should be to fix %SIG magic 
to always get and store to/from PL_warnhook and PL_diehook instead of 
only storing to it and hoping that nothing on the C level changes it (as 
perlio does).

My more general fear is that much of the magic isn't really transparent 
with regards to get/store, at least this is the second case where I 
think it will be better to simplify the code. The first time was with 
$$, which cached the current value and then had to "fix" the current 
value of $$ after fork() . Both cases broke when the assumptions broke, 
namely that the perl interpreter is the only one accessing C data 
structures...

Anyway, I'll submit the long-term fix for Perl soonish, which will go 
into #125439 , and the short-term patch to Marc to make it possible to 
have Coro on 5.22. The long term fix will be to remove code from Coro, 
at least for 5.24+, which I consider good.

Also, I have an SVG file with "Perl %SIG Magic Mystery Tour", which 
outlines the use of the magic API and field/struct usage for "S" and "s" 
magic. I hope to expand this to all kinds of magic, or maybe to turn 
this into a grant project to have more documentation on how Perl uses 
its internal APIs...

-max

Thread Previous | Thread Next


nntp.perl.org: Perl Programming lists via nntp and http.
Comments to Ask Bjørn Hansen at ask@perl.org | Group listing | About