Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix a segfault related to async statements #60

Closed
wants to merge 1 commit into from

Conversation

chazmcgarvey
Copy link

Hi friends,

After upgrading to DBD::Pg 3.10.0, my async app now dies on "Segmentation fault" or "double free or corruption". The exact place it dies is inconsistent, but capturing various backtraces reveals it's always in PQclear. So it seems clear we're attempting to free a result more than once. Here's a sample backtrace:

#0  0x00007fe110a45a90 in free () from /lib64/libc.so.6                                                               
#1  0x00007fe10d30f9fe in PQclear () from /usr/lib64/postgresql-11/lib64/libpq.so.5                                   
#2  0x00007fe10d358089 in _result () from .../lib/perl5/x86_64-linux/auto/DBD/Pg/Pg.so
#3  0x00007fe10d35994c in pg_st_deallocate_statement.isra () from .../lib/perl5/x86_64-linux/auto/DBD/Pg/Pg.so
#4  0x00007fe10d365023 in pg_st_destroy () from .../lib/perl5/x86_64-linux/auto/DBD/Pg/Pg.so
#5  0x00007fe10d352ea6 in XS_DBD__Pg__st_DESTROY () from .../lib/perl5/x86_64-linux/auto/DBD/Pg/Pg.so
#6  0x00007fe10d3939b8 in XS_DBI_dispatch () from .../lib/perl5/x86_64-linux/auto/DBI/DBI.so
#7  0x000055c2a3321179 in Perl_pp_entersub ()
...

I bisected to b312efb (made prior to 3.9.0).

I took a stab at fixing the problem and came to this solution. This does alleviate the segfaults for me, though I'd need someone more familiar with this code to verify it's the right solution.

Although it crashes consistently in my complex app, I haven't yet succeeded in even reproducing the crash in a test file. :-(

@@ -684,6 +684,8 @@ void dbd_db_destroy (SV * dbh, imp_dbh_t * imp_dbh)
if (imp_dbh->async_sth->result) {
TRACE_PQCLEAR;
PQclear(imp_dbh->async_sth->result);
if (imp_dbh->last_result == imp_dbh->async_sth->result)
Copy link
Author

@chazmcgarvey chazmcgarvey Sep 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this first change does nothing to alleviate segfaults for me, but looks safer to me to do this...

@a6502
Copy link

a6502 commented Oct 9, 2019

Hi,

I just ran into the same double free issue in an application that uses async extensively and can confirm that the above patch fixes that.

I don't have a small test-case but at least I can show the code: https://github.com/a6502/jobcenter/blob/master/bin/maestro

This uses Mojo::Pg (subclassed for better connection pooling).

Is there anything I can do to help solve this issue?

@chazmcgarvey
Copy link
Author

chazmcgarvey commented Oct 9, 2019

Yeah, my app also uses Mojo::Pg.

I considered that maybe Mojo::Pg was misusing DBD::Pg somehow, but after digging through the dbdpg code I settled on the flaw being here. What do you think @turnstep ?

@johto
Copy link

johto commented Oct 9, 2019

Maybe a duplicate of #37?

@chazmcgarvey
Copy link
Author

Looks very similar, but I didn't think it was the exact same bug because I bisected this crash to a commit made later than that bug was filed (b312efb).

@esabol
Copy link
Contributor

esabol commented Apr 17, 2020

Merge conflict. Needs rebasing?

@chazmcgarvey
Copy link
Author

Merge conflict. Needs rebasing?

Actually this PR should be abandoned because the issue was fixed with the 3.10.1 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants