Re: Minor issue with ping

On Fri, 18 Jun 2010, John Scoles wrote:

> The way Ping works is different is depending on the version of
> DBD::Oracle you are using.  Which version of DBD::Oracle are you using?? 
> 
Our "production" version is a bit behind (1.16), which is where problem
was first noticed.  Tried with version 1.23, and saw same issues.  Tried
supplied patch on 1.23, and problem fixed.  Got latest svn release (1.24)
just before sending email, and ping code looked the same (I admit did not
install or test on 1.24).

I see that ping() code has changed a bit, from doing a perl "select SYSDATE
from DUAL" to using xs code ora_ping(), but the "problem" that I see remains
and is essentially the same in all three versions I examined.
The basic problem is this:

sub ping {
 	my $ ($dbh) = @_;
         my $ok = 0;

 	eval {
 		some block of code to do the ping
 	};
 	return ($@) ? 0: $ok;
}

The eval block changes the global variable $@; either setting it to an
error code if the eval block fails, or clearing it if it succeeds.
Either way, if I have some code

eval {
 	my $obj = Some::Object->new;
 	die "Throw an exception";
      };
print "error is $@\n";

if the Some::Object object has a destructor which for some reason calls
DBD::Oracle ping, $@ is modified and I lose the information about the
exception I threw.   In particular, if the ping succeeds, I get an exception
with $@ unset, which the standard and common
 	eval { some code }; if ( $@ ) { handle exception }
sequence will miss the exception (admittedly a flaw of the sequence), but
even when done correctly leaves your catch block without any knowledge of what
the exception was.   (CGI::Session with Oracle datastore is a real world
example of a package which calls ping() in its destruction process.)

ping() works, but its side effect of resetting the global variable $@
which I consider "impolite" and can interfere with other packages.

If you just put a
 	local $@;
before the eval block in ping(), ping should continue to work as before, with
the exception that the global variable $@ is not altered by the call to
ping.  The clobbering of $@ is a side effect of current ping() routine that
I do not see mentioned in either DBI or DBD::Oracle pods, and does not seem
desirable to me.  Unless $@ is supposed to contain diagnostics on why the
ping failed, but then it should be documented.

Again, this is a small issue (using perl and DBD::Oracle for many years before
encountering this issue), but also very subtle and difficult to track when it
does (it confused several smart people for a couple of days). 
This change would make DBD::Oracle play better with others.



> cheers
> John Scoles
> 
> On Thu, Jun 17, 2010 at 2:34 PM, Thomas M. Payerle <[email protected]>
> wrote:
>       Hi,
>
>       My colleagues and I encountered a problem in some code which
>       seems
>       to be due to some impolite behavior on the part of the ping
>       routine
>       in DBD::Oracle.
>
>       Basically, we had an eval block with a locally declared (my)
>       CGI::Session
>       object using Oracle DB for storing session info.  When we
>       raise an exception,
>       the CGI::Session is destroyed, which somewhere results in
>       DBD::Oracle ping
>       being called.  ping() does not localize $@ for its eval
>       block, thereby
>       clobbering the exception text in $@.
>
>       I believe adding a "local $@" in the ping routine resolves
>       this issue
>       without any ill effect on the routine, as shown in attached
>       patch.
>       Not really a bug, but I believe this is better behavior.
>
>       Tom Payerle
>       OIT-TSS-DCS                             [email protected]
>       University of Maryland                  (301) 405-6135
>       College Park, MD 20742-4111
>
>       PS: I just wanted to offer my gratitude to the DBD::Oracle
>       developers for
>       their fine work on this module.
> 
>
>  --
> Catch Alex & Sheeri at ODTUG/Kaleidoscope - June 27 - July 1. 
> Hear Sheeri speak or email [email protected] to meet with Pythian.
> 
>

Tom Payerle
OIT-TSS-DCS				[email protected]
University of Maryland			(301) 405-6135
College Park, MD 20742-4111