From @ribasushi

Created by ribasushi@cpan.org

5.13.1 introduced an improved way of $@​ handling, by delaying the
$@​ assignment to the end of the eval{} block [1]. While "localized"
$@​ is a godsend, the current implementation makes it imposible to
examine (not overwrite) the value of $@​ *within* a DESTROY block.

An example of affected code is DBIx​::Class​::Storage​::TxnScopeGuard
which warns about uncommitted DBI transactions unless the destruction
is due to an exception (in which case the transaction will be rolled
back anyway) [2].

[1] http​://perl5.git.perl.org/perl.git/commitdiff/9d5401cef7488142a773a2e8980f9763da4a2cae?hp=9e5bbba0de25c01ae9355c7a97e237602a37e9f3
[2] http​://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits/DBIx-Class.git;a=blob;f=lib/DBIx/Class/Storage/TxnScopeGuard.pm;h=7f93113ac7ea53025b22a8177305493539c88989;hb=HEAD#l33

Flags:
    category=core
    severity=high

Site configuration information for perl 5.13.2:

Configured by rabbit at Mon Jul  5 15:34:37 CEST 2010.

Summary of my perl5 (revision 5 version 13 subversion 2) configuration:
   
  Platform:
    osname=linux, osvers=2.6.29.2.thes2, archname=i686-linux-thread-multi
    uname='linux thesaurus 2.6.29.2.thes2 #1 preempt tue sep 8 11:27:43 cest 2009 i686 gnulinux '
    config_args='-de -Dprefix=/home/rabbit/perl5/perlbrew/perls/5.13.2 -Dusethreads -Dusedevel'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=define, usemultiplicity=define
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=undef, use64bitall=undef, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.4.4', gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=4, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib /usr/lib64
    libs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=/lib/libc-2.11.1.so, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.11.1'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'

Locally applied patches:
    


@INC for perl 5.13.2:
    /home/rabbit/devel/utils/perl
    /home/rabbit/devel/utils/perl
    /home/rabbit/perl5/perlbrew/perls/5.13.2/lib/site_perl/5.13.2/i686-linux-thread-multi
    /home/rabbit/perl5/perlbrew/perls/5.13.2/lib/site_perl/5.13.2
    /home/rabbit/perl5/perlbrew/perls/5.13.2/lib/5.13.2/i686-linux-thread-multi
    /home/rabbit/perl5/perlbrew/perls/5.13.2/lib/5.13.2
    .


Environment for perl 5.13.2:
    HOME=/home/rabbit
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LC_ADDRESS=en_US.UTF-8
    LC_COLLATE=en_US.UTF-8
    LC_CTYPE=en_US.UTF-8
    LC_IDENTIFICATION=en_US.UTF-8
    LC_MESSAGES=en_US.UTF-8
    LC_MONETARY=en_US.UTF-8
    LC_NAME=en_US.UTF-8
    LC_NUMERIC=en_US.UTF-8
    LC_TELEPHONE=en_US.UTF-8
    LC_TIME=en_US.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/rabbit/devel/utils:/home/rabbit/perl5/perlbrew/bin:/home/rabbit/perl5/perlbrew/perls/current/bin:/home/rabbit/devel/utils:/home/rabbit/perl5/perlbrew/bin:/home/rabbit/perl5/perlbrew/perls/current/bin:/home/rabbit/bin:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/games
    PERL5LIB=/home/rabbit/devel/utils/perl:/home/rabbit/devel/utils/perl:
    PERL_AUTOINSTALL_PREFER_CPAN=1
    PERL_BADLANG (unset)
    SHELL=/bin/bash

From zefram@fysh.org

Peter Rabbitson wrote​:

            the current implementation makes it imposible to

examine (not overwrite) the value of $@​ *within* a DESTROY block.

An example of affected code is DBIx​::Class​::Storage​::TxnScopeGuard

To achieve what you want from this was already impossible. You could see
the exception being thrown when one is thrown, but during unwinding from
a local exit you were not guaranteed any particular value in $@​. In that
case $@​ would just have whatever value it had before the local exit.
What's changed is that $@​ now has its previous value in both situations.

TxnScopeGuard is essentially doing this​:

$ perl5.10.0 -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval { my $guard = bless({}); 1; };'
in DESTROY​: $@​=""
$ perl5.10.0 -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval { my $guard = bless({}); die "foo"; };'
in DESTROY​: $@​="foo at -e line 1.
"
$ perl5.10.0 -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval { my $guard = bless({}); eval { die "bar" }; die "foo"; };'
in DESTROY​: $@​="foo at -e line 1.
"
$

But it already loses in this situation​:

$ perl5.10.0 -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval { my $guard = bless({}); eval { die "bar"; }; 1; };'
in DESTROY​: $@​="bar at -e line 1.
"
$

It now loses in more situations​:

$ bleadperl -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval { my $guard = bless({}); 1; };'
in DESTROY​: $@​=""
$ bleadperl -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval { my $guard = bless({}); die "foo"; };'
in DESTROY​: $@​=""
$ bleadperl -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval { my $guard = bless({}); eval { die "bar"; }; 1; };'
in DESTROY​: $@​="bar at -e line 1.
"
$ bleadperl -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval { my $guard = bless({}); eval { die "bar"; }; die "foo"; };'
in DESTROY​: $@​="bar at -e line 1.
"
$

which warns about uncommitted DBI transactions unless the destruction
is due to an exception (in which case the transaction will be rolled
back anyway) [2].

You already have the correct logic for deciding to roll back​: you roll
back unless there was an explicit commit. So the only thing depending
on exceptionness is the warning. I'm afraid I don't have any good
suggestion for what to do about the warning.

-zefram

The RT System itself - Status changed from 'new' to 'open'

From @ribasushi

On Fri Jul 09 02​:48​:33 2010, zefram@​fysh.org wrote​:

Peter Rabbitson wrote​:

            the current implementation makes it imposible to

examine (not overwrite) the value of $@​ *within* a DESTROY block.

An example of affected code is DBIx​::Class​::Storage​::TxnScopeGuard

To achieve what you want from this was already impossible. You could
see
the exception being thrown when one is thrown, but during unwinding
from
a local exit you were not guaranteed any particular value in $@​. In
that
case $@​ would just have whatever value it had before the local exit.

When you say "unwinding" - do you mean out-of-order exit() destruction?
If yes - this is handled by enforcing proper order via a clever closure
in an END{}.

What's changed is that $@​ now has its previous value in both
situations.

TxnScopeGuard is essentially doing this​:

$ perl5.10.0 -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; }
eval { my $guard = bless({}); 1; };'
in DESTROY​: $@​=""
$ perl5.10.0 -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; }
eval { my $guard = bless({}); die "foo"; };'
in DESTROY​: $@​="foo at -e line 1.
"
$ perl5.10.0 -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; }
eval { my $guard = bless({}); eval { die "bar" }; die "foo"; };'
in DESTROY​: $@​="foo at -e line 1.
"
$

But it already loses in this situation​:

$ perl5.10.0 -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; }
eval { my $guard = bless({}); eval { die "bar"; }; 1; };'
in DESTROY​: $@​="bar at -e line 1.
"
$

This particular situation is a user error - he did not Try​::Tiny or
whatever his eval{}

It now loses in more situations​:

$ bleadperl -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval
{ my $guard = bless({}); 1; };'
in DESTROY​: $@​=""
$ bleadperl -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval
{ my $guard = bless({}); die "foo"; };'
in DESTROY​: $@​=""
$ bleadperl -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval
{ my $guard = bless({}); eval { die "bar"; }; 1; };'
in DESTROY​: $@​="bar at -e line 1.
"
$ bleadperl -le 'sub DESTROY { print "in DESTROY​: \$\@​=\"$@​\""; } eval
{ my $guard = bless({}); eval { die "bar"; }; die "foo"; };'
in DESTROY​: $@​="bar at -e line 1.
"
$

which warns about uncommitted DBI transactions unless the destruction
is due to an exception (in which case the transaction will be rolled
back anyway) [2].

You already have the correct logic for deciding to roll back​: you roll
back unless there was an explicit commit. So the only thing depending
on exceptionness is the warning. I'm afraid I don't have any good
suggestion for what to do about the warning.

The whole point of the guard *is* the warning. The rollback will happen
on $dbh destruction anyway, it just attempts to rollback in the guard to
warn of potential issues earlier.

In essence we will have to deprecate this module asap if it is certain
that a DESTROY block will have no knowledge of an existing exception in
future versions of perl.

Note - we care *whether* the DESTROY is a result of an exception, not
*what* the exception was. If there is some sort of way to determine this
(a sibling of $^S) or some sort of minimal XS module - all my problems
go away.

From @ribasushi

Stalling pending investigation of how to work around this without
touching the perl core.

@rgs - Status changed from 'open' to 'rejected'

From @ribasushi

Greetings everyone,

I would like to rehash the consequences of commit 96d9b9c made back on
2010-04-20. While originally the problem manifested itself as failing
tests within DBIx​::Class, it later became apparent that the change
deprives perl of an entire class of object-based guards.

I will try to briefly summarize the "what's so special" part below,
however for more info I recommend reading the full write-up from the
pov of DBIx​::Class at​:
http​://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits/DBIx-Class.git;a=blob_plain;f=useful_guard_objects.html;hb=refs/heads/txn_guard_breakage

So the TLDR version is​: If you have a guard object whose DESTROY has
important consequences (e.g. eats your data) - it is extremely useful
to be able to tell if an object is destroyed due to simply passing out
of scope or due to an exception-induced jump. If the destruction is not
caused by an exception, the chance is very high that the data is eaten
due to a programming error, and not notifying the user of this fact can
and will introduce nearly impossible to track bugs. For an explanation
of why would someone want to use an object for something so sensitive to
control-flow - read the link above.

I am all for the actual change (it's now impossible to clobber $@​
accidentally \o/), the problem is in the way this fix was implemented.
There undoubtedly is a way to make intermediate frames aware that an
exception is taking place. While I don't know much about the perl
internals something as simple as setting $@​ *twice* could be sufficient
to resolve this (while unfortunately keeping the caveats Zefram listed
above). If a new mechanism is implemented signaling to DESTROY why is it
being called - that's even better! Then the corner cases are also
solved, since any residual value of $@​ from previous eval{}s no longer
matters.

Cheers

From @ribasushi

So about a year has passed, and I'd like to rehash this RT once again. I
figured this recent writeup is a good thing to share with perl5-porters.
Comments welcome!

http​://www.perlmonks.org/?node_id=924524

Cheers