The described bug that is prevented by removing the quoting sounds exactly like somebody has a bug in their ref count management in their XS code.
- There is no XS code involved
- it makes no sense to have the quoting in the first place. When was the last time you added a component of an object by double-quoting instead of simply putting it there? It's unorthodox. It doesnt make sense. And it causes a bug.
| [reply] |
There is no XS code involved
Uh, I bet DBI is "involved" which usually means quite a large amount of XS code. Moose is mentioned which pulls in a ton of other modules, several of which involve XS code.
And I already noted that XS bugs have a tendency (IMO, IME) to impact things that don't appear at all related:
probably one of the major contributors to the rather frequent problems I see with "weird bugs" that just "go away" when some XS module stops being used (even though the XS module doesn't even seem related to the "weird bug").
(emphasis added)
it makes no sense to have the quoting in the first place
Several people appear to disagree with you on that point. If you want to argue against them, then perhaps you should address their points rather than throw out just "it makes no sense" (to you) and "it's unorthodox" (in your experience).
Several objects are being used as keys to hashes. It is very similar to "inside out object" techniques. Yes, "inside out objects" were rather unorthodox until they became rather mainstream and even a widely adopted "best practice" (and have fallen out of favor a bit since, IME).
So I don't find it unacceptable to keep this key around. Nor do I object to avoiding the creation of circular references (or even just avoiding some risk of circular references). Hash keys are just strings so "$self" is how you get the hash key.
And it CAUSES a bug.
Well, for some less useful definitions of "causes", I guess. It certainly isn't the root cause of the bug, IMO. Based on the somewhat vague description of the bug, it sounds to me like a bug that should be impossible to create just in Perl code.
It should be impossible for one to write Perl code such that
my $x= get();
my $y= $x->method();
# vs.
my $z= get()->method();
produce different results.
So I stand by my assertion that there must be a real bug in some non-Perl code (either XS code or Perl's own C code -- XS code being the much more likely case) for this to happen (though the problem statement is vague enough that I could easily be misunderstanding).
So it would be good for somebody to do some more debugging in hopes of finding or at least narrowing down the location of this other bug.
The code of DBIx::Simple is complex enough that I was not able to quickly understand the interplay of object life cycles. I have no confidence that changing "$self" to $self won't "CAUSE" bugs for other users of that module.
So it would be good to provide some complete, stand-alone code that reproduces this problem so it can at least be added to the test suite for the module (probably marked "to-do" for now).
If somebody makes the effort to understand the interplay of object life cycles, then it would be good to add some test cases around such things as well. Then we might even be able to point at a specific case that would be broken by the proposed change. Or be able to point at the tests around how life cycles are supposed to interact and how the proposed change doesn't break any of those tests.
| [reply]
[d/l]
[select] |
So it would be good to provide some complete, stand-alone code that reproduces this problem so it can at least be added to the test suite for the module (probably marked "to-do" for now).
OK here it is. (github link may be preferred because perlmonks wraps the code). And thank you for your polite reply to my issue. As long as others can confirm what I'm demonstrating, and then fix it, by getting rid of the quoting on line 165, then I will report it as a bug
{
package Util;
sub dbconnect {
use DBI;
DBI->connect('dbi:SQLite:temp.db');
}
}
{
package Local::DBIx::Simple::Q;
use Moose;
has 'q' => ( is => 'rw', default => sub { $main::backgroundqueue }
+ );
has 'standard' => ( is => 'rw', default => 0 );
use Data::Dumper;
sub BUILD {
my ($self) = @_;
$main::globalstandardconnection = $self->standard
}
sub enq {
my ( $self, @arg ) = @_;
warn sprintf "Enqueing with id %d this data: %s", $self->enq_i
+d,
Dumper( \@arg );
$self->q->enqueue( [ $self->enq_id, @arg ] );
}
}
{
package Local::DBIx::Simple;
use Moose;
extends qw(Local::DBIx::Simple::Q);
use DBIx::Simple;
has 'enq_id' => ( is => 'rw', default => 5 );
has 'deq_id' => ( is => 'rw', default => 6 );
sub dbh {
Util::dbconnect;
}
sub dbs {
my ($self) = @_;
my $dbs = DBIx::Simple->connect( $self->dbh );
}
}
{
package main;
use strict;
use warnings;
use Data::Dumper;
use Test::More;
use lib 'lib';
sub constructor {
Local::DBIx::Simple->new( standard => 0 );
}
sub create_database {
my ($dbh) = @_;
my $ddl = <<'EODDL';
create table table_one (
col1 integer not null primary key,
col2 TEXT
)
EODDL
$dbh->do($ddl);
}
sub main {
my $dbh = Util::dbconnect;
create_database($dbh);
my $Q = "SELECT * FROM table_one";
my $desired_class = 'DBIx::Simple::Statement';
my $desired_desc = "object isa $desired_class";
{ # CASE 1 - successful
my $s = constructor;
my $dbs = DBIx::Simple->connect( $s->dbh );
my $r = $dbs->query($Q);
warn sprintf 'Result of DBIx::Simple query: %s', Dumper($r
+);
my $h = $r->hashes;
warn sprintf 'Hashes? %s', Dumper($h);
ok( $r->{st}->isa($desired_class), $desired_desc );
}
{ # CASE 2 - successful
my $s = constructor;
my $dbs = $s->dbs;
my $r = $dbs->query($Q);
warn sprintf 'Result of DBIx::Simple-from-Local query: %s'
+,
Dumper($r);
my $h = $r->hashes;
warn sprintf 'Hashes? %s', Dumper($h);
ok( $r->{st}->isa($desired_class), $desired_desc );
}
{ # CASE 3 - *FAILS* when $self is quoted on line 165
my $s = constructor;
my $r = $s->dbs->query($Q);
ok( $r->{st}->isa($desired_class), $desired_desc );
}
}
}
main() unless caller;
1;
| [reply]
[d/l] |