wsutil: Remove flawed ws_pipe_close() function

The semantics behind ws_pipe_close() were broken since its introduction.
Forcing process termination on Windows, while simply setting variable on
other systems results in more OS specific code sprinkled all over the
place instead of less. Moreover ws_pipe_close() never handled standard
file handles. It is really hard to come up with sensible ws_pipe_close()
replacement, as process exit is actually asynchronous action. It is
recommended to register child watch using g_child_watch_add() instead.

Do not call ws_pipe_close() when deleting capture interface. Things will
break if extcap is still running when interface opts are being freed and
terminating process won't help.

Rework maxmind shutdown to rely on GIOChannel state. For unknown reason
TerminateProcess() is still needed on Windows. The actual root cause
should be identified and fixed instead of giving up hope that it will
ever work correctly on Windows. In other words, TerminateProcess()
should not be used as a pattern, but rather as a last resort.

1 files changed, 16 insertions, 17 deletions

diff --git a/epan/maxmind_db.c b/epan/maxmind_db.c
index b128ae370e..b9a662e1b0 100644
--- a/epan/maxmind_db.c
+++ b/epan/maxmind_db.c
@@ -171,12 +171,6 @@ write_mmdbr_stdin_worker(gpointer data _U_) {
     MMDB_DEBUG("starting write worker");
 
     while (1) {
-        if (!mmdbr_pipe_valid()) {
-            // Should be due to mmdb_resolve_stop.
-            MMDB_DEBUG("invalid mmdbr stdin pipe. exiting thread.");
-            return NULL;
-        }
-
         // On some operating systems (most notably macOS), g_async_queue_timeout_pop
         // will return immediately if we've been built with an older version of GLib:
         //   https://bugzilla.gnome.org/show_bug.cgi?id=673607
@@ -184,10 +178,13 @@ write_mmdbr_stdin_worker(gpointer data _U_) {
         // mmdb_resolve_stop will close our pipe and then push an invalid address
         // (mmdbr_stop_sentinel) onto the queue.
         char *request = (char *) g_async_queue_pop(mmdbr_request_q);
-        if (!request || strcmp(request, mmdbr_stop_sentinel) == 0) {
-            g_free(request);
+        if (!request) {
             continue;
         }
+        if (strcmp(request, mmdbr_stop_sentinel) == 0) {
+            g_free(request);
+            return NULL;
+        }
 
         MMDB_DEBUG("write %s ql %d", request, g_async_queue_length(mmdbr_request_q));
         status = g_io_channel_write_chars(mmdbr_pipe.stdin_io, request, strlen(request), &bytes_written, &err);
@@ -248,12 +245,8 @@ read_mmdbr_stdout_worker(gpointer data _U_) {
                 if (bytes_read > 0) {
                     bytes_in_buffer += bytes_read;
                 } else {
-                    if (!mmdbr_pipe_valid()) {
-                        // Should be due to mmdb_resolve_stop.
-                        MMDB_DEBUG("invalid mmdbr stdout pipe. exiting thread.");
-                        break;
-                    }
-                    MMDB_DEBUG("no pipe data");
+                    MMDB_DEBUG("no pipe data. exiting thread.");
+                    break;
                 }
             } else {
                 MMDB_DEBUG("long line");
@@ -387,9 +380,6 @@ static void mmdb_resolve_stop(void) {
 
     g_rw_lock_writer_lock(&mmdbr_pipe_mtx);
 
-    MMDB_DEBUG("closing pid %d", mmdbr_pipe.pid);
-    ws_pipe_close(&mmdbr_pipe);
-
     g_async_queue_push(mmdbr_request_q, g_strdup(mmdbr_stop_sentinel));
 
     g_rw_lock_writer_unlock(&mmdbr_pipe_mtx);
@@ -401,6 +391,15 @@ static void mmdb_resolve_stop(void) {
     MMDB_DEBUG("closing stdin IO");
     g_io_channel_unref(mmdbr_pipe.stdin_io);
 
+#ifdef _WIN32
+    /* TODO: Actually solve the issue instead of just terminating process */
+    MMDB_DEBUG("terminating pid %d", mmdbr_pipe.pid);
+    TerminateProcess(mmdbr_pipe.pid, 0);
+#endif
+    MMDB_DEBUG("closing pid %d", mmdbr_pipe.pid);
+    g_spawn_close_pid(mmdbr_pipe.pid);
+    mmdbr_pipe.pid = WS_INVALID_PID;
+
     // child process notices broken stdin pipe and exits (breaks stdout pipe)
     // read_mmdbr_stdout_worker should exit