Skip to content

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

Open
@roavin

Description

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

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

Labels

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions