[w32handle] Replace mono_w32handle_{ref,unref} by mono_w32handle_{duplicate, close...

* [w32handle] Fix handle dumped ref

* [w32handle] Replace exposing mono_w32handle_{ref,unref} by mono_w32handle_{duplicate,close}, it is closer to what the win32 API exposes.

* [w32handle] Remove unrefing of handles on shutdown

If one of the handle is a file handle for example, then w32file has already be cleaned up, so we cannot guarantee that the handle we are going to close is not going to call into w32file code.

diff --git a/mono/metadata/w32file-unix.c b/mono/metadata/w32file-unix.c
index 64e46c2d588487defd3ac2979aab5cdfc51d5396..d87335f2f0d3880d8a21f2593393f4694de1e824 100644 (file)
--- a/mono/metadata/w32file-unix.c
+++ b/mono/metadata/w32file-unix.c
@@ -3624,7 +3624,7 @@ mono_w32file_find_close (gpointer handle)
        mono_w32handle_unlock_handle (handle);
        
        MONO_ENTER_GC_SAFE;
-       mono_w32handle_unref (handle);
+       mono_w32handle_close (handle);
        MONO_EXIT_GC_SAFE;
        
        return(TRUE);
@@ -3995,7 +3995,7 @@ mono_w32file_create_pipe (gpointer *readpipe, gpointer *writepipe, guint32 size)
                g_warning ("%s: error creating pipe write handle", __func__);
 
                MONO_ENTER_GC_SAFE;
-               mono_w32handle_unref (read_handle);
+               mono_w32handle_close (read_handle);
                
                close (filedes[0]);
                close (filedes[1]);

diff --git a/mono/metadata/w32file.c b/mono/metadata/w32file.c
index 919965c1639ff43e809e0b4373ada759c7126413..296a84d6a47055af2eddaecfb48ce827f1b93eb0 100644 (file)
--- a/mono/metadata/w32file.c
+++ b/mono/metadata/w32file.c
@@ -1073,8 +1073,7 @@ ves_icall_System_IO_MonoIO_DuplicateHandle (HANDLE source_process_handle, HANDLE
        ret=DuplicateHandle (source_process_handle, source_handle, target_process_handle, target_handle, access, inherit, options);
        MONO_EXIT_GC_SAFE;
 #else
-       mono_w32handle_ref (source_handle);
-       *target_handle = source_handle;
+       *target_handle = mono_w32handle_duplicate (source_handle);
        ret = TRUE;
 #endif
 

diff --git a/mono/metadata/w32handle-namespace.c b/mono/metadata/w32handle-namespace.c
index 6975c48934b2057e05085f0b65505247bcf934f0..f45d2efae532dc9487ed5719201516f42a87acaa 100644 (file)
--- a/mono/metadata/w32handle-namespace.c
+++ b/mono/metadata/w32handle-namespace.c
@@ -91,9 +91,7 @@ mono_w32handle_namespace_search_handle_callback (gpointer handle, gpointer data,
                                __func__, handle);
 
                        /* we do not want the handle to be destroyed before we return it  */
-                       mono_w32handle_ref (handle);
-
-                       search_data->ret = handle;
+                       search_data->ret = mono_w32handle_duplicate (handle);
                }
 
                return TRUE;

diff --git a/mono/metadata/w32handle.c b/mono/metadata/w32handle.c
index 0dcfb6f70b4d90e415e0cb5ab3cdccd21ed349dd..02aa4db826306b53f6affa76beffbd683e4f0ea0 100644 (file)
--- a/mono/metadata/w32handle.c
+++ b/mono/metadata/w32handle.c
@@ -210,6 +210,12 @@ mono_w32handle_unlock_signal_mutex (void)
        mono_os_mutex_unlock (&global_signal_mutex);
 }
 
+static void
+mono_w32handle_ref (gpointer handle);
+
+static void
+mono_w32handle_unref (gpointer handle);
+
 void
 mono_w32handle_lock_handle (gpointer handle)
 {
@@ -297,29 +303,11 @@ mono_w32handle_init (void)
 void
 mono_w32handle_cleanup (void)
 {
-       int i, j, k;
+       int i;
 
        g_assert (!shutting_down);
        shutting_down = TRUE;
 
-       /* Every shared handle we were using ought really to be closed
-        * by now, but to make sure just blow them all away.  The
-        * exiting finalizer thread in particular races us to the
-        * program exit and doesn't always win, so it can be left
-        * cluttering up the shared file.  Anything else left over is
-        * really a bug.
-        */
-       for(i = SLOT_INDEX (0); private_handles[i] != NULL; i++) {
-               for(j = SLOT_OFFSET (0); j < HANDLE_PER_SLOT; j++) {
-                       MonoW32HandleBase *handle_data = &private_handles[i][j];
-                       gpointer handle = GINT_TO_POINTER (i*HANDLE_PER_SLOT+j);
-
-                       for(k = handle_data->ref; k > 0; k--) {
-                               mono_w32handle_unref (handle);
-                       }
-               }
-       }
-
        for (i = 0; i < SLOT_MAX; ++i)
                g_free (private_handles [i]);
 }
@@ -490,12 +478,44 @@ gpointer mono_w32handle_new_fd (MonoW32HandleType type, int fd,
        return(GUINT_TO_POINTER(fd));
 }
 
+static gboolean
+mono_w32handle_ref_core (gpointer handle, MonoW32HandleBase *handle_data);
+
+static gboolean
+mono_w32handle_unref_core (gpointer handle, MonoW32HandleBase *handle_data);
+
+static void
+w32handle_destroy (gpointer handle);
+
+gpointer
+mono_w32handle_duplicate (gpointer handle)
+{
+       MonoW32HandleBase *handle_data;
+
+       if (handle == INVALID_HANDLE_VALUE)
+               return handle;
+       if (!mono_w32handle_lookup_data (handle, &handle_data))
+               return INVALID_HANDLE_VALUE;
+       if (handle == (gpointer) 0 && handle_data->type != MONO_W32HANDLE_CONSOLE)
+               return handle;
+
+       if (!mono_w32handle_ref_core (handle, handle_data))
+               g_error ("%s: failed to ref handle %p", __func__, handle);
+
+       return handle;
+}
+
 gboolean
 mono_w32handle_close (gpointer handle)
 {
+       MonoW32HandleBase *handle_data;
+       gboolean destroy;
+
        if (handle == INVALID_HANDLE_VALUE)
                return FALSE;
-       if (handle == (gpointer) 0 && mono_w32handle_get_type (handle) != MONO_W32HANDLE_CONSOLE) {
+       if (!mono_w32handle_lookup_data (handle, &handle_data))
+               return FALSE;
+       if (handle == (gpointer) 0 && handle_data->type != MONO_W32HANDLE_CONSOLE) {
                /* Problem: because we map file descriptors to the
                 * same-numbered handle we can't tell the difference
                 * between a bogus handle and the handle to stdin.
@@ -504,7 +524,10 @@ mono_w32handle_close (gpointer handle)
                return FALSE;
        }
 
-       mono_w32handle_unref (handle);
+       destroy = mono_w32handle_unref_core (handle, handle_data);
+       if (destroy)
+               w32handle_destroy (handle);
+
        return TRUE;
 }
 
@@ -529,15 +552,6 @@ mono_w32handle_lookup (gpointer handle, MonoW32HandleType type,
        return(TRUE);
 }
 
-static gboolean
-mono_w32handle_ref_core (gpointer handle, MonoW32HandleBase *handle_data);
-
-static gboolean
-mono_w32handle_unref_core (gpointer handle, MonoW32HandleBase *handle_data);
-
-static void
-w32handle_destroy (gpointer handle);
-
 void
 mono_w32handle_foreach (gboolean (*on_each)(gpointer handle, gpointer data, gpointer user_data), gpointer user_data)
 {
@@ -644,14 +658,13 @@ mono_w32handle_unref_core (gpointer handle, MonoW32HandleBase *handle_data)
        return new == 0;
 }
 
-void mono_w32handle_ref (gpointer handle)
+static void
+mono_w32handle_ref (gpointer handle)
 {
        MonoW32HandleBase *handle_data;
 
-       if (!mono_w32handle_lookup_data (handle, &handle_data)) {
-               mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_W32HANDLE, "%s: failed to ref handle %p, unknown handle", __func__, handle);
-               return;
-       }
+       if (!mono_w32handle_lookup_data (handle, &handle_data))
+               g_error ("%s: failed to ref handle %p, unknown handle", __func__, handle);
 
        if (!mono_w32handle_ref_core (handle, handle_data))
                g_error ("%s: failed to ref handle %p", __func__, handle);
@@ -703,17 +716,14 @@ w32handle_destroy (gpointer handle)
 }
 
 /* The handle must not be locked on entry to this function */
-void
+static void
 mono_w32handle_unref (gpointer handle)
 {
        MonoW32HandleBase *handle_data;
        gboolean destroy;
 
-       if (!mono_w32handle_lookup_data (handle, &handle_data)) {
-               mono_trace (G_LOG_LEVEL_DEBUG, MONO_TRACE_W32HANDLE, "%s: failed to unref handle %p, unknown handle",
-                       __func__, handle);
-               return;
-       }
+       if (!mono_w32handle_lookup_data (handle, &handle_data))
+               g_error ("%s: failed to unref handle %p, unknown handle", __func__, handle);
 
        destroy = mono_w32handle_unref_core (handle, handle_data);
        if (destroy)
@@ -1077,13 +1087,14 @@ signal_handle_and_unref (gpointer handle)
        mono_os_cond_broadcast (cond);
        mono_os_mutex_unlock (mutex);
 
-       mono_w32handle_unref (handle);
+       mono_w32handle_close (handle);
 }
 
 static int
 mono_w32handle_timedwait_signal_handle (gpointer handle, guint32 timeout, gboolean poll, gboolean *alerted)
 {
        MonoW32HandleBase *handle_data;
+       gpointer handle_duplicate;
        int res;
 
        if (!mono_w32handle_lookup_data (handle, &handle_data))
@@ -1096,10 +1107,11 @@ mono_w32handle_timedwait_signal_handle (gpointer handle, guint32 timeout, gboole
                *alerted = FALSE;
 
        if (alerted) {
-               mono_thread_info_install_interrupt (signal_handle_and_unref, handle, alerted);
-               if (*alerted)
+               mono_thread_info_install_interrupt (signal_handle_and_unref, handle_duplicate = mono_w32handle_duplicate (handle), alerted);
+               if (*alerted) {
+                       mono_w32handle_close (handle_duplicate);
                        return 0;
-               mono_w32handle_ref (handle);
+               }
        }
 
        res = mono_w32handle_timedwait_signal_naked (&handle_data->signal_cond, &handle_data->signal_mutex, timeout, poll, alerted);
@@ -1107,8 +1119,8 @@ mono_w32handle_timedwait_signal_handle (gpointer handle, guint32 timeout, gboole
        if (alerted) {
                mono_thread_info_uninstall_interrupt (alerted);
                if (!*alerted) {
-                       /* if it is alerted, then the handle is unref in the interrupt callback */
-                       mono_w32handle_unref (handle);
+                       /* if it is alerted, then the handle_duplicate is closed in the interrupt callback */
+                       mono_w32handle_close (handle_duplicate);
                }
        }
 
@@ -1124,7 +1136,7 @@ dump_callback (gpointer handle, gpointer handle_specific, gpointer user_data)
                g_error ("cannot dump unknown handle %p", handle);
 
        g_print ("%p [%7s] signalled: %5s ref: %3d ",
-               handle, mono_w32handle_ops_typename (handle_data->type), handle_data->signalled ? "true" : "false", handle_data->ref);
+               handle, mono_w32handle_ops_typename (handle_data->type), handle_data->signalled ? "true" : "false", handle_data->ref - 1 /* foreach increase ref by 1 */);
        mono_w32handle_ops_details (handle_data->type, handle_data->specific);
        g_print ("\n");
 

diff --git a/mono/metadata/w32handle.h b/mono/metadata/w32handle.h
index 029f24dad82d3135cccc00d71e162b2d78d27424..b7df9a8810e50fbd9a1400cbdf9e443e0cbf732e 100644 (file)
--- a/mono/metadata/w32handle.h
+++ b/mono/metadata/w32handle.h
@@ -116,6 +116,9 @@ mono_w32handle_new (MonoW32HandleType type, gpointer handle_specific);
 gpointer
 mono_w32handle_new_fd (MonoW32HandleType type, int fd, gpointer handle_specific);
 
+gpointer
+mono_w32handle_duplicate (gpointer handle);
+
 gboolean
 mono_w32handle_close (gpointer handle);
 
@@ -134,12 +137,6 @@ mono_w32handle_foreach (gboolean (*on_each)(gpointer handle, gpointer data, gpoi
 void
 mono_w32handle_dump (void);
 
-void
-mono_w32handle_ref (gpointer handle);
-
-void
-mono_w32handle_unref (gpointer handle);
-
 void
 mono_w32handle_register_capabilities (MonoW32HandleType type, MonoW32HandleCapability caps);
 

diff --git a/mono/metadata/w32mutex-unix.c b/mono/metadata/w32mutex-unix.c
index 189b808ba22436aace20b71b1449db5ca1838f88..ab2ef003ddf9db938f2479bb5612d9657666b993 100644 (file)
--- a/mono/metadata/w32mutex-unix.c
+++ b/mono/metadata/w32mutex-unix.c
@@ -38,8 +38,6 @@ mono_w32mutex_open (const gchar* utf8_name, gint32 right G_GNUC_UNUSED, gint32 *
 static void
 thread_own_mutex (MonoInternalThread *internal, gpointer handle)
 {
-       mono_w32handle_ref (handle);
-
        /* if we are not on the current thread, there is a
         * race condition when allocating internal->owned_mutexes */
        g_assert (mono_thread_internal_is_current (internal));
@@ -47,7 +45,7 @@ thread_own_mutex (MonoInternalThread *internal, gpointer handle)
        if (!internal->owned_mutexes)
                internal->owned_mutexes = g_ptr_array_new ();
 
-       g_ptr_array_add (internal->owned_mutexes, handle);
+       g_ptr_array_add (internal->owned_mutexes, mono_w32handle_duplicate (handle));
 }
 
 static void
@@ -61,7 +59,7 @@ thread_disown_mutex (MonoInternalThread *internal, gpointer handle)
        removed = g_ptr_array_remove (internal->owned_mutexes, handle);
        g_assert (removed);
 
-       mono_w32handle_unref (handle);
+       mono_w32handle_close (handle);
 }
 
 static void

diff --git a/mono/metadata/w32process-unix.c b/mono/metadata/w32process-unix.c
index 559939c9b61722492b2738121a798ca189c8a6e9..972942af2b60817e509e536df483ad7b5015cc99 100644 (file)
--- a/mono/metadata/w32process-unix.c
+++ b/mono/metadata/w32process-unix.c
@@ -736,7 +736,7 @@ processes_cleanup (void)
                if (process->signalled && process->handle) {
                        /* This process has exited and we need to remove the artifical ref
                         * on the handle */
-                       mono_w32handle_unref (process->handle);
+                       mono_w32handle_close (process->handle);
                        process->handle = NULL;
                }
        }
@@ -862,7 +862,7 @@ mono_w32process_init (void)
        process_set_name (&process_handle);
 
        current_process = mono_w32handle_new (MONO_W32HANDLE_PROCESS, &process_handle);
-       g_assert (current_process);
+       g_assert (current_process != INVALID_HANDLE_VALUE);
 
        mono_os_mutex_init (&processes_mutex);
 }
@@ -965,8 +965,7 @@ get_process_foreach_callback (gpointer handle, gpointer handle_specific, gpointe
        if (mono_w32handle_issignalled (handle))
                return FALSE;
 
-       mono_w32handle_ref (handle);
-       foreach_data->handle = handle;
+       foreach_data->handle = mono_w32handle_duplicate (handle);
        return TRUE;
 }
 
@@ -2019,8 +2018,7 @@ process_create (const gunichar2 *appname, const gunichar2 *cmdline,
 
                /* Keep the process handle artificially alive until the process
                 * exits so that the information in the handle isn't lost. */
-               mono_w32handle_ref (handle);
-               process->handle = handle;
+               process->handle = mono_w32handle_duplicate (handle);
 
                mono_os_mutex_lock (&processes_mutex);
                process->next = processes;