mojolicious / mojo-pg

Mojolicious :heart: PostgreSQL
https://metacpan.org/release/Mojo-Pg
Artistic License 2.0
101 stars 46 forks source link

Callbacks calling ->unlisten breaks #47

Closed pipcet closed 6 years ago

pipcet commented 6 years ago

Steps to reproduce the behavior

Register two different callbacks for the same notification, the first of which calls $pubsub->unlisten($chan, $cb) in the notification handler.

Expected behavior

Both callbacks should be called. (That's what I expected. There's no note in the documentation that callbacks mustn't call ->unlisten.)

#!/usr/bin/env perl
use utf8;
use Mojolicious::Lite;
use Mojo::Pg;
use Mojo::Pg::PubSub;
use Mojo::UserAgent;
use Mojo::JSON qw(decode_json encode_json);
use Time::HiRes qw(time);
use File::Slurp qw(read_file write_file);
use File::Temp qw(tempdir);

helper pg => sub {
    state $pg = Mojo::Pg->new(...);
    $pg;
};

get '/test' => sub {
    my $c = shift;
    my $pg = $c->pg;
    my $db = $pg->db;
    my $pubsub = $pg->pubsub;

    my $cb;
    $cb = sub {
        my $pubsub = shift;

        warn "$cb called";

        $pubsub->unlisten(test => $cb);
    };

    $pubsub->listen(test => $cb);
    $pubsub->listen(test => $cb);
    $pubsub->notify(test =>);
};

app->start;

Actual behavior

Only the first callback is called.

It modifies @{$pubsub->{chans}{$chan}}, which confuses the for loop in Mojo::Pg::PubSub::_db.

PR forthcoming.

pipcet commented 6 years ago

Oops, messed up the example.

#!/usr/bin/env perl
use utf8;
use Mojolicious::Lite;
use Mojo::Pg;
use Mojo::Pg::PubSub;

helper pg => sub {
    state $pg = Mojo::Pg->new('postgresql://pip@%2fvar%2frun%2fpostgresql:5433/mojolicious');
    $pg;
};

get '/test' => sub {
    my $c = shift;
    my $pg = $c->pg;
    my $db = $pg->db;
    my $pubsub = $pg->pubsub;

    my $cb;
    $cb = sub {
        my $pubsub = shift;

        warn;

        $pubsub->unlisten(test => $cb);
    };

    $pubsub->listen(test => $cb);
    $pubsub->listen(test => sub { warn; });
    $pubsub->notify(test =>);
};

app->start;

Expected behavior: two warnings. Actual behavior: a single warning. Fixed by PR.