libpq is not thread safe

When postgreSQL is compiled with --thread-safe that libpq should be
thread safe. But it is not true when somebody call fork(). The problem
is that fork() forks only active threads and some mutex can stay locked
by another thread. We use ssl_config mutex which is global.

We need implement atfork handlers to fix this. See
http://www.opengroup.org/onlinepubs/009695399/functions/pthread_atfork.html

We should add pthread_atfork into _ini libpq section.

Another problem with fork is that new process inherit connections and so
on. Which is not also good, but it is happened also on single threaded
application and developer can fix it in own code. Maybe some notice in
documentation should help what application should do after fork.

Comments?

Zdenek

Zdenek Kotala wrote:
> When postgreSQL is compiled with --thread-safe that libpq should be
> thread safe. But it is not true when somebody call fork(). The problem
> is that fork() forks only active threads and some mutex can stay locked
> by another thread. We use ssl_config mutex which is global.
>
> We need implement atfork handlers to fix this. See
> http://www.opengroup.org/onlinepubs/009695399/functions/pthread_atfork.html
>
> We should add pthread_atfork into _ini libpq section.
>
> Another problem with fork is that new process inherit connections and so
> on. Which is not also good, but it is happened also on single threaded
> application and developer can fix it in own code. Maybe some notice in
> documentation should help what application should do after fork.

Yep, added to TODO list:

Make libpq thread-safe in programs that use fork()

This requires the use of pthread_atfork() to release global locks
held in libpq

* http://archives.postgresql.org/pgsql-hackers/2009-04/msg00747.php
--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM> writes:
> When postgreSQL is compiled with --thread-safe that libpq should be
> thread safe. But it is not true when somebody call fork(). The problem
> is that fork() forks only active threads and some mutex can stay locked
> by another thread. We use ssl_config mutex which is global.

fork() without exec() when there are open libpq connections is
unbelievably dangerous anyway --- you will have multiple processes
that all think they own the same database connection. I think writing
code to deal with this for the ssl_config mutex is entirely a waste
of time.

regards, tom lane

Tom Lane píše v ne 03. 05. 2009 v 16:39 -0400:
> Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM> writes:
> > When postgreSQL is compiled with --thread-safe that libpq should be
> > thread safe. But it is not true when somebody call fork(). The problem
> > is that fork() forks only active threads and some mutex can stay locked
> > by another thread. We use ssl_config mutex which is global.
>
> fork() without exec() when there are open libpq connections is
> unbelievably dangerous anyway --- you will have multiple processes
> that all think they own the same database connection.

The difference is that developer can close connection, but he is not
able to unlock a lock after fork. OK libpq does not offer any PQFinish
variant which frees only resources and close connection without
terminate message, but he can do it with dirty hacking.

Another advantage of atfork handler is that you can close all open
connection and clean resource (similar to what pkcs11 library does). But
at this moment libpq does not keep list of open connections and atfork
handler works only with pthreads.

> I think writing code to deal with this for the ssl_config mutex is entirely a waste
> of time.

yeah, I prefer to document it together how to deal with fork() and
sessions.

Zdenek

Zdenek Kotala wrote:
>
> Tom Lane p??e v ne 03. 05. 2009 v 16:39 -0400:
> > Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM> writes:
> > > When postgreSQL is compiled with --thread-safe that libpq should be
> > > thread safe. But it is not true when somebody call fork(). The problem
> > > is that fork() forks only active threads and some mutex can stay locked
> > > by another thread. We use ssl_config mutex which is global.
> >
> > fork() without exec() when there are open libpq connections is
> > unbelievably dangerous anyway --- you will have multiple processes
> > that all think they own the same database connection.
>
> The difference is that developer can close connection, but he is not
> able to unlock a lock after fork. OK libpq does not offer any PQFinish
> variant which frees only resources and close connection without
> terminate message, but he can do it with dirty hacking.
>
> Another advantage of atfork handler is that you can close all open
> connection and clean resource (similar to what pkcs11 library does). But
> at this moment libpq does not keep list of open connections and atfork
> handler works only with pthreads.
>
> > I think writing code to deal with this for the ssl_config mutex is entirely a waste
> > of time.
>
> yeah, I prefer to document it together how to deal with fork() and
> sessions.

Done, patch attached and applied.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> > Another advantage of atfork handler is that you can close all open
> > connection and clean resource (similar to what pkcs11 library does). But
> > at this moment libpq does not keep list of open connections and atfork
> > handler works only with pthreads.
> >
> > > I think writing code to deal with this for the ssl_config mutex is entirely a waste
> > > of time.
> >
> > yeah, I prefer to document it together how to deal with fork() and
> > sessions.
>
> Done, patch attached and applied.

I went with a <warning> because it seemed most appropriate, but it looks
very large:

http://developer.postgresql.org/pgdocs/postgres/libpq-connect.html

Should it be a notice?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian píše v čt 28. 05. 2009 v 17:20 -0400:

> >
> > Done, patch attached and applied.
>
> I went with a <warning> because it seemed most appropriate, but it looks
> very large:
>
> http://developer.postgresql.org/pgdocs/postgres/libpq-connect.html
>
> Should it be a notice?

I prefer warning. It is important message and beginners usually don't
know it.

Zdenek

Zdenek Kotala wrote:
>
> Bruce Momjian p??e v ?t 28. 05. 2009 v 17:20 -0400:
>
> > >
> > > Done, patch attached and applied.
> >
> > I went with a <warning> because it seemed most appropriate, but it looks
> > very large:
> >
> > http://developer.postgresql.org/pgdocs/postgres/libpq-connect.html
> >
> > Should it be a notice?
>
> I prefer warning. It is important message and beginners usually don't
> know it.

OK.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +