memprof + threads fix ...



So,

	A bit more poking - and it turns out all the memprof thread stuff
assumes that 'getpid' returns something different for each thread ;-) of
course, with NPTL [ ie. any recent system ] that just isn't so.

	Using a __thread variable instead makes it really solid.

	I attach a new patch; Owen - who maintains this now ? - and/or can one
just commit such things that fix clear brokenness without overmuch
review ? :-)

	Thanks,

		Michael.

-- 
 michael meeks novell com  <><, Pseudo Engineer, itinerant idiot
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/memprof/ChangeLog,v
retrieving revision 1.129
diff -u -r1.129 ChangeLog
--- ChangeLog	1 Apr 2005 23:09:59 -0000	1.129
+++ ChangeLog	14 Jun 2005 10:50:00 -0000
@@ -1,3 +1,23 @@
+2005-06-14  Michael Meeks  <michael meeks novell com>
+
+	* intercept.c (allocate_thread, find_thread):
+	update signatures and re-work for NPTL which gives
+	the same getpid() for each process: fixes all the
+	threading problems.
+
+	* process.c (input_func): reset the info structure
+	between messages; defensively.
+
+2005-06-13  Michael Meeks  <michael meeks novell com>
+
+	* server.c (ensure_cleanup, mp_server_init),
+	* process.c (process_start_input): use a fuller set
+	of G_IO_ flags for various conditions.
+
+	* intercept.c (exit_wait, mi_write_stack): use atomic
+	increment for seqno - if we get out of step process.c
+	bombs with an ever-growing sorted GList.
+
 2005-04-01  Steve Murphy  <murf e-tools com>
 
         * configure.in: Added "rw" to ALL_LINGUAS.
Index: intercept.c
===================================================================
RCS file: /cvs/gnome/memprof/intercept.c,v
retrieving revision 1.5
diff -u -r1.5 intercept.c
--- intercept.c	3 Sep 2004 22:29:47 -0000	1.5
+++ intercept.c	14 Jun 2005 10:50:01 -0000
@@ -46,7 +46,9 @@
  */
 
 typedef struct {
+#ifdef NPTL
 	uint32_t ref_count;
+#endif
 	pid_t pid;
 	int outfd;
 	pid_t clone_pid;	/* See comments in clone_thunk */
@@ -72,10 +74,10 @@
 
 static ThreadInfo threads[MAX_THREADS];
 static char *socket_path = NULL;
 static char socket_buf[64];
-static unsigned int seqno = 0;
+static uint32_t seqno = 0;
 
 #undef ENABLE_DEBUG
 
 #ifdef ENABLE_DEBUG
 #define MI_DEBUG(arg) mi_debug arg
@@ -87,72 +89,85 @@
 #define MI_DEBUG(arg) (void)0
 #endif /* ENABLE_DEBUG */
 
+/*
+ * For the new NPTL implementation, all threads return the
+ * same 'getpid' - so instead we use a thread variable.
+ */
+#define NPTL
+
+#ifdef NPTL
+static __thread ThreadInfo global_per_thread = { 0, };
+#endif
+
 static ThreadInfo *
-allocate_thread (pid_t pid)
+allocate_thread (void)
 {
+	ThreadInfo *thread = NULL;
+#ifdef NPTL
+	thread = &global_per_thread;
+#else
 	int i;
 	
 	for (i = 0; i < MAX_THREADS; i++) {
 		if (threads[i].ref_count == 0) {
 			unsigned int new_count = mi_atomic_increment (&threads[i].ref_count);
 			if (new_count == 1) {
-				threads[i].pid = pid;
-				threads[i].clone_pid = 0;
-				return &threads[i];
+				thread = &threads[i];
+				break;
 			} else
 				mi_atomic_decrement (&threads[i].ref_count);
 		}
 	}
-	
-	mi_debug ("Can't find free thread slot");
-	tracing = MI_FALSE;
-	_exit(1);
+	if (!thread) {
+		mi_debug ("Can't find free thread slot");
+		tracing = MI_FALSE;
+		_exit(1);
+	}
+#endif	
+	thread->pid = getpid();
+	thread->clone_pid = 0;
 
-	return NULL;
+	return thread;
 }
 
 static void
 release_thread (ThreadInfo *thread)
 {
 	thread->pid = 0;
+#ifndef NPTL
 	mi_atomic_decrement (&thread->ref_count);
+#endif
 }
 
 static ThreadInfo *
-find_thread (pid_t pid)
+find_thread (void)
 {
-	int i;
-#if 1
 	ThreadInfo *thread;
+#ifdef NPTL
+	thread = &global_per_thread;
 #else
-	/* Over-optimized GCC/GLibc extension happy version
-	 * Requires gcc-3.2 and glibc-2.3.
-	 */
-	static __thread ThreadInfo *thread = NULL;
 	int i;
-
-a	if (__builtin_expect (thread == NULL, 0))
-		return thread;
-#endif	
+	pid_t pid = getpid();
 
 	for (i=0; i < MAX_THREADS; i++)
 		if (threads[i].pid == pid) {
 			thread = &threads[i];
-
-			if (thread->clone_pid) {
-				/* See comments in clone_thunk() */
-				new_process (thread, thread->clone_pid, MI_CLONE);
-				thread->clone_pid = 0;
-			}
-
-			return thread;
+			break;
 		}
+#endif
+	if (!thread) {
+			mi_debug ("Thread not found\n");
+			tracing = MI_FALSE;
+			_exit(1);
+	}
 
-	mi_debug ("Thread not found\n");
-	tracing = MI_FALSE;
-	_exit(1);
-
-	return NULL;
+	if (thread->clone_pid) {
+			/* See comments in clone_thunk() */
+			new_process (thread, thread->clone_pid, MI_CLONE);
+			thread->clone_pid = 0;
+	}
+	
+	return thread;
 }
 
 static void
@@ -249,7 +264,7 @@
 	info.fork.seqno = 0;
 
 	if (!thread)
-		thread = allocate_thread (info.fork.new_pid);
+			thread = allocate_thread ();
 
 	thread->outfd = outfd;
 
@@ -331,9 +346,9 @@
 
 	info->alloc.stack_size = n_frames;
 	info->alloc.pid = getpid();
-	info->alloc.seqno = seqno++;
+	info->alloc.seqno = mi_atomic_increment (&seqno) - 1;
 
-	thread = find_thread (info->alloc.pid);
+	thread = find_thread ();
 	
 	if (!mi_write (thread->outfd, info, sizeof (MIInfo)) ||
 	    !mi_write (thread->outfd, frames, n_frames * sizeof(void *)))
@@ -352,7 +367,7 @@
 		int pid;
 		int old_pid = getpid();
 
-		find_thread (old_pid); /* Make sure we're registered */
+		find_thread (); /* Make sure we're registered */
 		
 		pid = (*old_fork) ();
 
@@ -380,7 +395,7 @@
 		int pid;
 		int old_pid = getpid();
 
-		find_thread (old_pid); /* Make sure we're registered */
+		find_thread (); /* Make sure we're registered */
 		
 		pid = (*old_vfork) ();
 
@@ -434,7 +449,7 @@
 	 * So, we simply allocate the structure for the thread and delay
 	 * the initialization.
 	 */
-	thread = allocate_thread (getpid());
+	thread = allocate_thread ();
 	thread->clone_pid = data->pid;
 	data->started = 1;
 
@@ -452,14 +467,14 @@
 
 	if (!mi_check_init ())
 		abort_unitialized ("__clone");
-
+	
 	if (tracing) {
 		data.started = 0;
 		data.fn = fn;
 		data.arg = arg;
 		data.pid = getpid();
 		
-		find_thread (data.pid); /* Make sure we're registered */
+		find_thread (); /* Make sure we're registered */
 		
 		pid = (*old_clone) (clone_thunk, child_stack, flags, (void *)&data, xarg1, xarg2, xarg3, xarg4);
 
@@ -490,12 +505,12 @@
 	int count;
 	char response;
 	info.any.operation = MI_EXIT;
-	info.any.seqno = seqno++;
+	info.any.seqno = mi_atomic_increment (&seqno) - 1;
 	info.any.pid = getpid();
 	
 	mi_stop ();
 	
-	thread = find_thread (info.any.pid);
+	thread = find_thread ();
 	
 	if (mi_write (thread->outfd, &info, sizeof (MIInfo)))
 		/* Wait for a response before really exiting
Index: process.c
===================================================================
RCS file: /cvs/gnome/memprof/process.c,v
retrieving revision 1.37
diff -u -r1.37 process.c
--- process.c	13 Sep 2004 18:43:52 -0000	1.37
+++ process.c	14 Jun 2005 10:50:02 -0000
@@ -600,6 +600,8 @@
 	MPProcess *process = NULL;
   
 	do {
+		memset (&info, 0, sizeof (info));
+		info.operation = -1;
 		count = 0;
 		if (g_io_channel_get_flags(source) & G_IO_FLAG_IS_READABLE)
 			g_io_channel_read_chars (source, (char *)&info, sizeof(info), &count, NULL);
@@ -621,7 +623,9 @@
 			    info.operation == MI_TIME) {
 				void **stack_buffer = NULL;
 				StackStash *stash = get_stack_stash (input_process);
-			
+
+				g_assert (count >= sizeof (MIInfoAlloc));
+
 				stack_buffer = g_alloca (sizeof (void *) * info.alloc.stack_size);
 				g_io_channel_read_chars (source, (char *)stack_buffer, sizeof(void *) * info.alloc.stack_size, &count, NULL);
 				stack = stack_stash_store (stash, stack_buffer, info.alloc.stack_size);
@@ -667,7 +672,7 @@
 	if (!process->input_tag && process->input_channel)
 		process->input_tag = g_io_add_watch_full (process->input_channel,
 							  G_PRIORITY_LOW,
-							  G_IO_IN | G_IO_HUP,
+							  G_IO_IN | G_IO_PRI | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
 							  input_func, process, NULL);
 }
 
Index: server.c
===================================================================
RCS file: /cvs/gnome/memprof/server.c,v
retrieving revision 1.14
diff -u -r1.14 server.c
--- server.c	20 Sep 2003 21:02:18 -0000	1.14
+++ server.c	14 Jun 2005 10:50:03 -0000
@@ -164,7 +164,8 @@
 	channel = g_io_channel_unix_new (server->socket_fd);
 	g_io_channel_set_encoding (channel, NULL, NULL);
 	
-	server->control_watch = g_io_add_watch (channel, G_IO_IN | G_IO_HUP, control_func, server);
+	server->control_watch = g_io_add_watch (channel, G_IO_IN | G_IO_PRI | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+											control_func, server);
 	g_io_channel_unref (channel);
 
 	g_object_ref (G_OBJECT (server));
@@ -366,7 +367,7 @@
 		channel = g_io_channel_unix_new (terminate_pipe[0]);
 		g_io_channel_set_encoding (channel, NULL, NULL);
 		
-		g_io_add_watch (channel, G_IO_IN, terminate_io_handler, NULL);
+		g_io_add_watch (channel, G_IO_IN | G_IO_PRI, terminate_io_handler, NULL);
 		g_io_channel_unref (channel);
 		
 		added_cleanup = TRUE;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]