PR #2268 for Mojo::Promise leads to regression with Mojo::Pg::Database

[zaucker]

The pull request #2268 breaks the async methods insert_p, update_p, select_p, delete_p in one of our Mojolicious based applications.

The symptom is an error like

DBD::Pg::st fetchrow_hashref failed: no statement executing at /home/zaucker/checkouts/hin/hin-cred-mgr/hin-cred-mgr/t/../thirdparty/lib/perl5/Mojo/Pg/Results.pm line 22.

e.g. when doing something like

await $res = insert_p('table', \%fields, { returning => 'id });

I verified this with Mojo::Promise from v9.41 (working) and with the patch for AWAIT_ON_READY applied (breaking).

I will try create a small test program for this, but wanted to submit this issue before this already merged patch gets in the potential 9.42.

[TFBW]

Please do provide a test case, as I have no other way to detect the issue.

[TFBW]

My preliminary analysis leads me to suspect that the former memory leak issue was keeping some object referenced which is now being destroyed "correctly" but exposing the underlying problem. I'll need to analyse Mojo::Pg::Database for such a possibility. That's a different repo, though. Suggestions, @kraih ?

[zaucker]

As I said in the issue I will try to make a small Test asap. Just wanted to alert immediately to the problem in case of a imminent release. Am 06.09.2025 um 01:42 schrieb Brett Watson ***@***.***>:TFBW left a comment (mojolicious/mojo#2276) Please do provide a test case, as I have no other way to detect the issue. —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>

[oetiker]

My preliminary analysis leads me to suspect that the former memory leak issue was keeping some object referenced which is now being destroyed "correctly" but exposing the underlying problem. I'll need to analyse Mojo::Pg::Database for such a possibility. That's a different repo, though. Suggestions, @kraih ?

https://github.com/mojolicious/mojo-pg a pretty popular extension ... but yes what you describe might well be the root cause. Still we need to know. And you seem to have an eye for it :)

[TFBW]

Does Mojo-Pg fail any tests when run in conjunction with this fix?

[zaucker]

I am still trying to create a simple test case. Just a single insert_p() is not enough to reproduce the problem.

[zaucker]

Does Mojo-Pg fail any tests when run in conjunction with this fix?

AWAIT_ON_READY is not called by running the tests in Mojo::Pg.

[kraih]

Mojo::Pg does not fail any tests with #2268 applied. However, it does not contain tests using async/await either.

[kraih]

To me a user error looks most likely. Perhaps the database object goes out of scope? The Mojo::Pg code here is very simple and does not leave a lot of room for bugs.

[zaucker]

Mojo::Pg does not fail any tests with #2268 applied. However, it does not contain tests using async/await either.

And this is why AWAIT_ON_READY is not being called as mentioned above.

[zaucker]

To me a user error looks most likely. Perhaps the database object goes out of scope? The Mojo::Pg code here is very simple and does not leave a lot of room for bugs.

I cannot rule that out, yet. However, it is hard to understand how this could happen in these few lines of code:

async sub insert_cred_p ($c) {   
    my $db = $c->db; 

    my $client = (await $db->select_p(...))->hash;

    # some code here without any calls to anything

    my $fields = { .... }; # just a hash, no calls to anything in its construction
    my $ret;
    try {
        $ret = await $db->insert_p('cred', $fields, {returning => 'cred_id'});
    }
    catch ($err) {
        $c->log->error("couldn't insert credential for client $in->{client_id} ($token_group), $client_json - $err");
        return $c->render( status => 500, json => {error => 'internal error'} );
    }
    my $cred_id = $ret->hash->{cred_id};

So it seems, that $db is being "lost" between the insert to the table and getting the result from returning.

And the problem does not occur if $db->insert() is used instead of $db->insert_p().

[kraih]

And if you add something like undef $db; to the catch block it works? Or move the $db creation outside the function?

[TFBW]

If there's a problem with my patch, it's probably the assumption that AWAIT_ON_READY never gets called on already-resolved Promises. I've been unable to generate a test case where that happens, but it would likely be a problem if it did. Try patching Mojo::Promise so that AWAIT_ON_READY is as follows (one line added).

sub AWAIT_ON_READY {
  my ($self, $cb) = @_;
  push @{$self->{resolve}}, $cb;
  push @{$self->{reject}},  $cb;
  $self->_defer if $self->{results};
}

This code assumes that the right thing to do under the circumstances is the same kind of deferred execution that then would perform. Maybe it would be better to just invoke the callback immediately and not add it to the queues at all. Kinda hard to say! I'm not even sure whether that's the problem in any case, so flail around a bit with it and see what happens.

If that doesn't help, then perhaps you could try to provide us with a dump of what the Promises for select_p and insert_p look like at strategic points? I'm not sure what's been promised to whom in the overall scheme of things here.

[zaucker]

If there's a problem with my patch, it's probably the assumption that AWAIT_ON_READY never gets called on already-resolved Promises. I've been unable to generate a test case where that happens, but it would likely be a problem if it did. Try patching Mojo::Promise so that AWAIT_ON_READY is as follows (one line added).

sub AWAIT_ON_READY {
my ($self, $cb) = @_;
push @{$self->{resolve}}, $cb;
push @{$self->{reject}}, $cb;
$self->_defer if $self->{results};
}
This code assumes that the right thing to do under the circumstances is the same kind of deferred execution that then would perform. Maybe it would be better to just invoke the callback immediately and not add it to the queues at all. Kinda hard to say! I'm not even sure whether that's the problem in any case, so flail around a bit with it and see what happens.

Same failure.

If that doesn't help, then perhaps you could try to provide us with a dump of what the Promises for select_p and insert_p look like at strategic points? I'm not sure what's been promised to whom in the overall scheme of things here.

Will do in a separate comment.

[zaucker]

And if you add something like undef $db; to the catch block it works?

    my $ret;
    try {
        $ret = await $db->insert_p('cred', $fields, {returning => 'cred_id'});
    }
    catch ($err) {
        undef $db;
        return $c->render( status => 500, json => {error => 'internal error'} );
    }
    my $cred_id = $ret->hash->{cred_id};

This doesn't change the failure. Although I might have misunderstood what you meant.