libpq is not thread safe

Started by Zdenek Kotalaover 16 years ago8 messages
#1Zdenek Kotala
Zdenek.Kotala@Sun.COM

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

#2Bruce Momjian
bruce@momjian.us
In reply to: Zdenek Kotala (#1)
Re: libpq is not thread safe

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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Zdenek Kotala (#1)
Re: libpq is not thread safe

Zdenek Kotala <Zdenek.Kotala@Sun.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

#4Zdenek Kotala
Zdenek.Kotala@Sun.COM
In reply to: Tom Lane (#3)
Re: libpq is not thread safe

Tom Lane píše v ne 03. 05. 2009 v 16:39 -0400:

Zdenek Kotala <Zdenek.Kotala@Sun.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

#5Bruce Momjian
bruce@momjian.us
In reply to: Zdenek Kotala (#4)
1 attachment(s)
Re: libpq is not thread safe

Zdenek Kotala wrote:

Tom Lane p??e v ne 03. 05. 2009 v 16:39 -0400:

Zdenek Kotala <Zdenek.Kotala@Sun.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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

Attachments:

/rtmp/xtext/x-diffDownload
Index: doc/src/sgml/libpq.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/libpq.sgml,v
retrieving revision 1.288
diff -c -c -r1.288 libpq.sgml
*** doc/src/sgml/libpq.sgml	27 Apr 2009 16:27:36 -0000	1.288
--- doc/src/sgml/libpq.sgml	28 May 2009 20:01:37 -0000
***************
*** 64,69 ****
--- 64,79 ----
     whether a connection was successfully made before queries are sent
     via the connection object.
  
+    <warning>
+     <para>
+      On Unix, forking a process with open libpq connections can lead to
+      unpredictable results because the parent and child processes share
+      the same sockets and operating system resources.  For this reason,
+      such usage is not recommended, though doing an <function>exec</> from
+      the child process to load a new executable is safe.
+     </para>
+    </warning>
+ 
     <note>
      <para>
       On Windows, there is a way to improve performance if a single
#6Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#5)
Re: libpq is not thread safe

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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#7Zdenek Kotala
Zdenek.Kotala@Sun.COM
In reply to: Bruce Momjian (#6)
Re: libpq is not thread safe

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

#8Bruce Momjian
bruce@momjian.us
In reply to: Zdenek Kotala (#7)
Re: libpq is not thread safe

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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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