Bug fixes resulting from stress testing the process forking code with

Timeline.

2004-09-06  Dick Porter  <dick@ximian.com>

	* handles.c (_wapi_handle_unref): Reset the private record's type
	(_wapi_handle_process_fork): Fix long-standing bug in checking
	handle return values.  Also do the required bookkeeping with the
	new process's handles.

	* daemon.c: When creating a new process's handles, check whether
	the shared space needs to be increased

2004-09-06  Dick Porter  <dick@ximian.com>

	* process.c (ves_icall_System_Diagnostics_Process_Start_internal):
	Close the new process's thread handle, as we don't use it.  The
	handle stays around forever otherwise.

svn path=/branches/mono-1-0/mono/; revision=33407

diff --git a/mono/io-layer/ChangeLog b/mono/io-layer/ChangeLog
index fec9ca9fbe05d151ef388bf51eeb62058abbda0d..b3fbbc1a9768ae2f85ea702f248d8998b18a1016 100644 (file)
--- a/mono/io-layer/ChangeLog
+++ b/mono/io-layer/ChangeLog
@@ -1,3 +1,13 @@
+2004-09-06  Dick Porter  <dick@ximian.com>
+
+       * handles.c (_wapi_handle_unref): Reset the private record's type
+       (_wapi_handle_process_fork): Fix long-standing bug in checking
+       handle return values.  Also do the required bookkeeping with the
+       new process's handles.
+
+       * daemon.c: When creating a new process's handles, check whether
+       the shared space needs to be increased
+
 2004-07-22  Dick Porter  <dick@ximian.com>
 
        * timed-thread.c: 

diff --git a/mono/io-layer/daemon.c b/mono/io-layer/daemon.c
index 62aa75ce1f335d8a18b7e0fcbc90aa92ad9791c5..a2b124bc510aa2896e4f5746dc9ff195e642357e 100644 (file)
--- a/mono/io-layer/daemon.c
+++ b/mono/io-layer/daemon.c
@@ -830,23 +830,11 @@ static void send_reply (GIOChannel *channel, WapiHandleResponse *resp)
        _wapi_daemon_response (g_io_channel_unix_get_fd (channel), resp);
 }
 
-/*
- * process_new:
- * @channel: The client making the request
- * @channel_data: Our data for this channel
- * @type: type to init handle to
- *
- * Find a free handle and initialize it to 'type', increase refcnt and
- * send back a reply to the client.
- */
-static void process_new (GIOChannel *channel, ChannelData *channel_data,
-                        WapiHandleType type)
+static guint32 new_handle_with_shared_check (WapiHandleType type)
 {
-       guint32 handle;
-       WapiHandleResponse resp={0};
-       
-       handle=_wapi_handle_new_internal (type);
-       if(handle==0) {
+       guint32 handle = 0;
+
+       while ((handle = _wapi_handle_new_internal (type)) == 0) {
                /* Try and allocate a new shared segment, and have
                 * another go
                 */
@@ -871,15 +859,34 @@ static void process_new (GIOChannel *channel, ChannelData *channel_data,
                                        channels[i].open_handles=_wapi_g_renew0 (channels[i].open_handles, old_len, new_len);
                                }
                        }
-
-                       handle=_wapi_handle_new_internal (type);
                } else {
                        /* Map failed.  Just return 0 meaning "out of
                         * handles"
                         */
+                       break;
                }
        }
        
+       return(handle);
+}
+
+/*
+ * process_new:
+ * @channel: The client making the request
+ * @channel_data: Our data for this channel
+ * @type: type to init handle to
+ *
+ * Find a free handle and initialize it to 'type', increase refcnt and
+ * send back a reply to the client.
+ */
+static void process_new (GIOChannel *channel, ChannelData *channel_data,
+                        WapiHandleType type)
+{
+       guint32 handle;
+       WapiHandleResponse resp={0};
+       
+       handle = new_handle_with_shared_check (type);
+       
        /* handle might still be set to 0.  This is handled at the
         * client end
         */
@@ -1064,11 +1071,11 @@ static void process_process_fork (GIOChannel *channel, ChannelData *channel_data
         * client must check if either handle is 0 and take
         * appropriate error handling action.
         */
-       process_handle=_wapi_handle_new_internal (WAPI_HANDLE_PROCESS);
+       process_handle = new_handle_with_shared_check (WAPI_HANDLE_PROCESS);
        ref_handle (daemon_channel_data, process_handle);
        ref_handle (channel_data, process_handle);
        
-       thread_handle=_wapi_handle_new_internal (WAPI_HANDLE_THREAD);
+       thread_handle = new_handle_with_shared_check (WAPI_HANDLE_THREAD);
        ref_handle (daemon_channel_data, thread_handle);
        ref_handle (channel_data, thread_handle);
        
@@ -1246,7 +1253,7 @@ static void process_process_fork (GIOChannel *channel, ChannelData *channel_data
 
                resp.u.process_fork.pid=pid;
        }
-                       
+
        resp.u.process_fork.process_handle=process_handle;
        resp.u.process_fork.thread_handle=thread_handle;
 

diff --git a/mono/io-layer/handles.c b/mono/io-layer/handles.c
index 769235fe76d89584bcd65bc2fac0881b1a63f1fa..07fc8a31f9c8ea09cac3b2d495ff3c28caa71858 100644 (file)
--- a/mono/io-layer/handles.c
+++ b/mono/io-layer/handles.c
@@ -676,6 +676,7 @@ void _wapi_handle_unref (gpointer handle)
 
                _wapi_handle_ops_close_private (handle);
                _wapi_handle_get_shared_segment (segment)->handles[idx].type=WAPI_HANDLE_UNUSED;
+               _wapi_handle_get_private_segment (segment)->handles[idx].type=WAPI_HANDLE_UNUSED;
                
                /* Destroy the mutex and cond var.  We hope nobody
                 * tried to grab them between the handle unlock and
@@ -1284,6 +1285,10 @@ gboolean _wapi_handle_ops_isowned (gpointer handle)
  */
 gboolean CloseHandle(gpointer handle)
 {
+       if (handle == NULL) {
+               return(FALSE);
+       }
+
        _wapi_handle_unref (handle);
        
        return(TRUE);
@@ -1609,9 +1614,26 @@ gboolean _wapi_handle_process_fork (guint32 cmd, guint32 env, guint32 dir,
                 * exec_errno will be set, and the handle will be
                 * signalled immediately.
                 */
-               if(process_handle==0 || thread_handle==0) {
+               if(*process_handle==0 || *thread_handle==0) {
                        return(FALSE);
                } else {
+                       /* This call returns new handles, so we need to do
+                        * a little bookkeeping
+                        */
+                       if (_wapi_private_data != NULL) {
+                               guint32 segment, idx;
+
+                               _wapi_handle_segment (*process_handle,
+                                                     &segment, &idx);
+                               _wapi_handle_ensure_mapped (segment);
+                               _wapi_handle_get_private_segment (segment)->handles[idx].type = WAPI_HANDLE_PROCESS;
+
+                               _wapi_handle_segment (*thread_handle,
+                                                     &segment, &idx);
+                               _wapi_handle_ensure_mapped (segment);
+                               _wapi_handle_get_private_segment (segment)->handles[idx].type = WAPI_HANDLE_THREAD;
+                       }
+
                        return(TRUE);
                }
        } else {

diff --git a/mono/metadata/ChangeLog b/mono/metadata/ChangeLog
index 588d20d06fba6478b77ff0264133fce600954eff..68867811c169c34325c1f4c1b104fd7e1d84496e 100644 (file)
--- a/mono/metadata/ChangeLog
+++ b/mono/metadata/ChangeLog
@@ -1,3 +1,9 @@
+2004-09-06  Dick Porter  <dick@ximian.com>
+
+       * process.c (ves_icall_System_Diagnostics_Process_Start_internal):
+       Close the new process's thread handle, as we don't use it.  The
+       handle stays around forever otherwise.
+
 2004-09-05 Gonzalo Paniagua Javier <gonzalo@ximian.com>
 
        * assembly.c: provide more information when loading an assembly fails.

diff --git a/mono/metadata/process.c b/mono/metadata/process.c
index 90edc4023e544d0e7677441442faaaa3ae2bd690..5fa57bcf0ed71501f3c3101b9a4859d60d867eb6 100644 (file)
--- a/mono/metadata/process.c
+++ b/mono/metadata/process.c
@@ -875,7 +875,9 @@ MonoBoolean ves_icall_System_Diagnostics_Process_Start_internal (MonoString *app
 
        if(ret) {
                process_info->process_handle=procinfo.hProcess;
-               process_info->thread_handle=procinfo.hThread;
+               /*process_info->thread_handle=procinfo.hThread;*/
+               process_info->thread_handle=NULL;
+               CloseHandle(procinfo.hThread);
                process_info->pid=procinfo.dwProcessId;
                process_info->tid=procinfo.dwThreadId;
        } else {