Initialization deadlocks, triggered when using Apache MPM Worker (with patch)

[roavin]

DBD::mysql version

4.050

MySQL client version

8.0.41

Server version

8.0.41-0ubuntu0.22.04.1

Operating system version

Ubuntu 22.04.5 LTS

What happened?

Our server hosts a somewhat popular website. Occasionally (sometimes every few days, sometimes months between) we would get apache processes that chew through 100% CPU and require kill -9 to end. Analysing the core dumps showed us that the processes are stuck in MySQL client library internals in a weird, seemingly inexplicable state. I believe I figured out the issue, however:

DBD::mysql will, even in the newest version, call mysql_library_init() every time it connects. This isn't principally a problem, because that call does nothing if the library is already initialized. However, suppose Apache spawns a new process with two (or more) workers which both simultaneously connect to a database in separate threads. In that case, mysql_library_init() may be called simultaneously and start initializing itself twice, which would lead to the problems we've observed. The MySQL documentation states:

In a nonmultithreaded environment, the call to mysql_library_init() may be omitted, because mysql_init() invokes it automatically as necessary. However, mysql_library_init() is not thread-safe in a multithreaded environment, and thus neither is mysql_init(), which calls mysql_library_init(). You must either call mysql_library_init() prior to spawning any threads, or else use a mutex to protect the call, whether you invoke mysql_library_init() or indirectly through mysql_init(). Do this prior to any other client library call.

It's nearly impossible to reliably reproduce this issue, of course. I've written a patch on my local (non-production) machine and it doesn't appear to break anything, but obviously I can't tell for sure if this would actually solve the problem. The patch uses pthread_once() which should be available on pretty much any platform out there.

diff --git a/dbdimp.c b/dbdimp.c
index 39db033..35ebaab 100644
--- a/dbdimp.c
+++ b/dbdimp.c
@@ -57,6 +57,21 @@ typedef struct sql_type_info_s
     int is_num;
 } sql_type_info_t;

+/*
+  Ensure we only call mysql_library_init once, since that is not threadsafe.
+  Not doing this before had lead to crashes in Apache MPM workers.
+*/
+pthread_once_t once_mysql_initialized = PTHREAD_ONCE_INIT;
+
+static void init_mysql_library(void)
+{
+  mysql_library_init(0, NULL, NULL);
+}
+
+static void ensure_mysql_initialized()
+{
+  pthread_once(&once_mysql_initialized, init_mysql_library);
+}

 /*

@@ -1206,7 +1221,7 @@ MYSQL *mysql_dr_connect(
 #else
     client_flag = CLIENT_FOUND_ROWS;
 #endif
-    mysql_library_init(0, NULL, NULL);
+    ensure_mysql_initialized();
     mysql_init(sock);

     if (imp_dbh)
diff --git a/dbdimp.h b/dbdimp.h
index 3d8c76b..ec208da 100644
--- a/dbdimp.h
+++ b/dbdimp.h
@@ -22,7 +22,7 @@
 #include <mysql.h>  /* Comes with MySQL-devel */
 #include <mysqld_error.h>  /* Comes MySQL */
 #include <errmsg.h> /* Comes with MySQL-devel */
-
+#include <pthread.h>

 #define true 1
 #define false 0

Other information

No response

[dveeden]

Could you send the patch as pull request?

[dveeden]

This could be the same issue as #421

[roavin]

Could you send the patch as pull request?

#465

This could be the same issue as #421

Based on a quick skim, that seems possible.