Thread: BUG #3031: PHP (and others?) cannot close a connection when "in" a COPY-statement

The following bug has been logged online:

Bug reference:      3031
Logged by:          Arjen van der Meijden
Email address:      acmmailing@tweakers.net
PostgreSQL version: 8.2.1
Operating system:   Debian Linux (etch, 2.6.17 kernel)
Description:        PHP (and others?) cannot close a connection when "in" a
COPY-statement
Details:

We noticed that when issueing a 'die'-statement in php, compiled against
both 8.1 or 8.2 will not really die when just before the die-statement a
COPY-statement to postgresql was started. In stead it gets in some (busy
wait?) loop, consuming 100% cpu.

Here is some example code that triggers the behaviour:
<?
echo "Starting\n";
$conn = pg_connect('');
pg_query($conn, "CREATE TEMPORARY TABLE test (test integer);");
pg_query($conn, "COPY test FROM STDIN");
die("And now it hangs in with 100% cpu\n");
?>

Of course you'd expect a prompt to return, rather than the process consuming
all cpu and never returning.

I'm not sure whether this is a php-issue or a postgresql-client-api one, but
when connecting gdb to that process and doing a 'bt' it displays:
#0  0xa7c36d9f in free () from /lib/tls/libc.so.6
#1  0xa7c38c4f in malloc () from /lib/tls/libc.so.6
#2  0xa7f58bdb in PQmakeEmptyPGresult () from /usr/lib/libpq.so.5
#3  0xa7f59941 in PQgetResult () from /usr/lib/libpq.so.5
#4  0x08086fd0 in _close_pgsql_link (rsrc=0x81e42bc) at
/usr/src/php-4.4.2/ext/pgsql/pgsql.c:276
#5  0x08139fb2 in list_entry_destructor (ptr=0x81e42bc) at
/usr/src/php-4.4.2/Zend/zend_list.c:177
#6  0x08137977 in zend_hash_apply_deleter (ht=0x81a49e0, p=0x81e4284)
    at /usr/src/php-4.4.2/Zend/zend_hash.c:611
#7  0x08137b97 in zend_hash_graceful_reverse_destroy (ht=0x81a49e0)
    at /usr/src/php-4.4.2/Zend/zend_hash.c:677
#8  0x0812b9ed in shutdown_executor () at
/usr/src/php-4.4.2/Zend/zend_execute_API.c:211
#9  0x08133801 in zend_deactivate () at /usr/src/php-4.4.2/Zend/zend.c:689
#10 0x08107862 in php_request_shutdown (dummy=0x0) at
/usr/src/php-4.4.2/main/main.c:999
#11 0x0814ee56 in main (argc=2, argv=0xaf992f54) at
/usr/src/php-4.4.2/sapi/cli/php_cli.c:881

We can reproduce this on x86-boxes with recent php4 and the latest
php5(-cvs). I have not tested other sapi's than the cli-php versions but I
also tested it on a (amd64) gentoo box.

If it is a php-bug, perhaps it'd be usefull if one of you could comment
here: http://bugs.php.net/bug.php?id=40544

Best regards,

Arjen van der Meijden

"Arjen van der Meijden" <acmmailing@tweakers.net> writes:
> I'm not sure whether this is a php-issue or a postgresql-client-api one, but
> when connecting gdb to that process and doing a 'bt' it displays:
> #0  0xa7c36d9f in free () from /lib/tls/libc.so.6
> #1  0xa7c38c4f in malloc () from /lib/tls/libc.so.6
> #2  0xa7f58bdb in PQmakeEmptyPGresult () from /usr/lib/libpq.so.5
> #3  0xa7f59941 in PQgetResult () from /usr/lib/libpq.so.5
> #4  0x08086fd0 in _close_pgsql_link (rsrc=0x81e42bc) at
> /usr/src/php-4.4.2/ext/pgsql/pgsql.c:276

Given that stack trace, you'd better complain to the PHP folk.  There is
no loop in that path in libpq, so the looping must be occurring
somewhere in the PHP code.

            regards, tom lane

On 20-2-2007 4:42 Tom Lane wrote:
> "Arjen van der Meijden" <acmmailing@tweakers.net> writes:
>> I'm not sure whether this is a php-issue or a postgresql-client-api one, but
>> when connecting gdb to that process and doing a 'bt' it displays:
>> #0  0xa7c36d9f in free () from /lib/tls/libc.so.6
>> #1  0xa7c38c4f in malloc () from /lib/tls/libc.so.6
>> #2  0xa7f58bdb in PQmakeEmptyPGresult () from /usr/lib/libpq.so.5
>> #3  0xa7f59941 in PQgetResult () from /usr/lib/libpq.so.5
>> #4  0x08086fd0 in _close_pgsql_link (rsrc=0x81e42bc) at
>> /usr/src/php-4.4.2/ext/pgsql/pgsql.c:276
>
> Given that stack trace, you'd better complain to the PHP folk.  There is
> no loop in that path in libpq, so the looping must be occurring
> somewhere in the PHP code.

I had a look at that _close_pgsql_link-function and this it the code:

static void _close_pgsql_link(zend_rsrc_list_entry *rsrc TSRMLS_DC)
{
     PGconn *link = (PGconn *)rsrc->ptr;
     PGresult *res;

     while ((res = PQgetResult(link))) {
         PQclear(res);
     }
     PQfinish(link);
     PGG(num_links)--;
}

So there is indeed a very clear loop. But I'm not sure whether this code
is incorrect as a generic "close the connection cleanly without memory
leaks"-function? If I duplicate that while-loop to my c-test code it
indeed keeps getting a non-null/false result from PQgetResult and thus
will loop indefinitely.

Do you know a clean example that displays how to close a link cleanly,
including this COPY-situation? Or can you indicate which commands should
be added to do so? My knowledge of the C-api is very limited, so I don't
know what's wrong here.

Best regards,

Arjen

Arjen van der Meijden <acmmailing@tweakers.net> writes:
> I had a look at that _close_pgsql_link-function and this it the code:

>      while ((res = PQgetResult(link))) {
>          PQclear(res);
>      }

> So there is indeed a very clear loop. But I'm not sure whether this code
> is incorrect as a generic "close the connection cleanly without memory
> leaks"-function?

It's surely incorrect as a means of getting out of a COPY state.  To
tell the truth, if their intention is to close the connection
unceremoniously, they should just do that, ie PQfinish or just quit
the application.  So I'd suggest removing that loop.

            regards, tom lane

On 20-2-2007 8:07 Tom Lane wrote:
> It's surely incorrect as a means of getting out of a COPY state.  To
> tell the truth, if their intention is to close the connection
> unceremoniously, they should just do that, ie PQfinish or just quit
> the application.  So I'd suggest removing that loop.

I think its meant as a generic way of closing the connection. I.e. not
only to close the connection as a last-resort "clean things up before we
return to the webserver/console, but also half-way if the client asks
for it (as the code behind 'pg_close').

I.e. you don't want memory-leaks to build up because someone, for some
reason, needs something like this and the internal code doesn't clean up
something that easily could've been cleaned up:

while($something)
{
  $c = pg_connect("..");
  // Queries
  pg_close($c);
}

Although obviously, that kind of code is not so common and should only
be used if there is a very good reason not to open/close the connection
outside the loop. Besides that, its very common in php to just leave the
garbage to the php-engine, probably even worse than in Java.
I guess, that if they really wanted to keep something like this
while-loop, they need to request the result-status and if its not the
result of a finished query (either failed, ok, empty, etc) they should
send a cancel request?

Anyway, I'll add your advice to that bug-report.

Best regards,

Arjen