maxmind_db: Close our pipe FDs inside a lock.
authorGerald Combs <gerald@wireshark.org>
Fri, 4 Jan 2019 21:24:10 +0000 (13:24 -0800)
committerAnders Broman <a.broman58@gmail.com>
Sat, 5 Jan 2019 06:46:20 +0000 (06:46 +0000)
Lock our pipe mutex before closing its file descriptors. This should
hopefully fix some infrequent crashes that I'm seeing on my Windows 7 VM.

Add a note about GRWLock behavior on Windows which doesn't appear to be
related to this issue, but which is nevertheless important.

Ping-Bug: 14701
Change-Id: I32e66a24258264fa65a907f319755594f90c0177
Reviewed-on: https://code.wireshark.org/review/31375
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
epan/maxmind_db.c

index 6a68f7821e920d5937e72cec94f4ab1ab54b46df..1ba2c9fb12277cb4469c8b26c4dec8ae305d93c8 100644 (file)
@@ -43,6 +43,14 @@ static mmdb_lookup_t mmdb_not_found;
 
 static GThread *write_mmdbr_stdin_thread;
 static GAsyncQueue *mmdbr_request_q; // g_allocated char *
+// The GLib documentation says that g_rw_lock_reader_lock can be called
+// recursively:
+//   https://developer.gnome.org/glib/stable/glib-Threads.html#g-rw-lock-reader-lock
+// However, g_rw_lock_reader_lock calls AcquireSRWLockShared
+//   https://gitlab.gnome.org/GNOME/glib/blob/master/glib/gthread-win32.c#L206
+// and SRW locks "cannot be acquired recursively"
+//   https://docs.microsoft.com/en-us/windows/desktop/Sync/slim-reader-writer--srw--locks
+//   https://blogs.msdn.microsoft.com/oldnewthing/20160506-00/?p=93416
 static GRWLock mmdbr_pipe_mtx;
 
 // Hashes of mmdb_lookup_t
@@ -332,7 +340,7 @@ read_mmdbr_stdout_worker(gpointer data _U_) {
 
 /**
  * Stop our mmdbresolve process.
- * Can be called from any thread.
+ * Main thread only.
  */
 static void mmdb_resolve_stop(void) {
     char *request;
@@ -347,12 +355,15 @@ static void mmdb_resolve_stop(void) {
         return;
     }
 
-    ws_close(mmdbr_pipe.stdin_fd);
-    ws_close(mmdbr_pipe.stdout_fd);
-
     g_rw_lock_writer_lock(&mmdbr_pipe_mtx);
+
     MMDB_DEBUG("closing pid %d", mmdbr_pipe.pid);
     ws_pipe_close(&mmdbr_pipe);
+
+    MMDB_DEBUG("closing pipe FDs");
+    ws_close(mmdbr_pipe.stdin_fd);
+    ws_close(mmdbr_pipe.stdout_fd);
+
     g_rw_lock_writer_unlock(&mmdbr_pipe_mtx);
 
     g_thread_join(write_mmdbr_stdin_thread);