GNOME.org
 

Permission to commit ...

Hi there,

	The attached patches sort out the referncing mahem going on inside
linc & the giop code - at least, they work for me and stop the ORB leaking
like a sieve.

	Here is the problem they address:

	There are two ways to create a linc-connection,

		a) ORBit_object_get_connection
		b) linc_server_handle_io

	In case a) the client side:

		+ the connection can be closed by a HUP
		+ but the Object ref still has a pointer to the connection
		  so the connection cannot be freed.
		+ Consequently there is an 'invalidate connection' step
		  followed by a 'destroy connection' step when the Object
		  reference is released.

	In case b) the server side:

		+ the connection can be closed by a HUP, and at this point
		  it should release all the resources associated with it.

	My solution: [ a ref == a g_object_ref ]

		+ while the watch is connected [ waiting for an error
		  condition ] the connection has a ref on itself.
		+ the associated object reference also holds a ref

		+ when the watch is disconnected - such as eg. as the
		  connection is being invalidated, the connection is
		  unreffed
			- if case a) there is still a ref on the object
			  so, the connection is closed, but the pointer is
			  still valid.
			- if case b) there was only the self reference so
			  the connection is torn down cleanly.

	So, the attached patches implement this - and indeed memory usage
is nicely static. The patches are aginst the ORBit2 orbit-small branch and
linc HEAD. The issue is that committing the linc patch will break the [
already broken ] ORBit2 HEAD even further, so can you advise ?

	Thanks,

		Michael.

-- 
 mmeeks gnu org  <><, Pseudo Engineer, itinerant idiot

? elliot
? include/orbit/orb-core/orbit-interface.h
? src/orb/orb-core/orbit-interface.h
? src/orb/orb-core/runidl_iface
? test/everything/server.log
? test/everything/client.log
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/ORBit2/ChangeLog,v
retrieving revision 1.12.2.25
diff -u -r1.12.2.25 ChangeLog
--- ChangeLog	2001/05/24 11:58:11	1.12.2.25
+++ ChangeLog	2001/05/24 20:36:43
@@ -1,9 +1,25 @@
+2001-05-25  Michael Meeks  <michael ximian com>
+
+	* src/orb/orb-core/corba-context.c (CORBA_Context_set_one_value):
+	remove the hash table value, so we don't screw up the key allocation.
+
 2001-05-24  Michael Meeks  <michael ximian com>
 
+	* src/orb/GIOP/giop-connection.c (giop_connection_initiate): 
+	reference the connection before returning it, if it wasn't
+	looked up in the list.
+	(giop_connection_remove_by_orb): fix serious brokenness.
+	(giop_connection_close): publicise.
+	(giop_connection_shutdown): remove the connection from the
+	connection list.
+	(giop_connection_unref): remove last unref sillies.
+
 	* src/orb/orb-core/corba-object.c (g_CORBA_Object_equal): uglify,
 	we need to be able to scan for MULTIPLE_COMPONENTS profiles in
 	order to do the GIOP profile comparison (get_mci): impl helper.
 	(IOP_Profile_equal): similarly uglify.
+	(CORBA_Object_release_cb): close the connection and
+	unref it.
 
 	* src/orb/poa/poa.c (ORBit_POA_oid_to_ref): set the profile_type
 	on the iiop profile.
Index: include/orbit/GIOP/giop-connection.h
===================================================================
RCS file: /cvs/gnome/ORBit2/include/orbit/GIOP/giop-connection.h,v
retrieving revision 1.15
diff -u -r1.15 giop-connection.h
--- include/orbit/GIOP/giop-connection.h	2000/10/31 23:37:49	1.15
+++ include/orbit/GIOP/giop-connection.h	2001/05/24 20:36:43
@@ -53,5 +53,6 @@
 					 GIOPVersion giop_version);
 void giop_connection_unref(GIOPConnection *cnx);
 void giop_connection_remove_by_orb(gpointer match_orb_data);
+void giop_connection_close (GIOPConnection *cnx);
 
 #endif
Index: src/orb/GIOP/giop-connection.c
===================================================================
RCS file: /cvs/gnome/ORBit2/src/orb/GIOP/giop-connection.c,v
retrieving revision 1.19.2.3
diff -u -r1.19.2.3 giop-connection.c
--- src/orb/GIOP/giop-connection.c	2001/05/24 10:12:18	1.19.2.3
+++ src/orb/GIOP/giop-connection.c	2001/05/24 20:36:43
@@ -23,6 +23,7 @@
 static void
 giop_connection_list_add(GIOPConnection *cnx)
 {
+  g_return_if_fail (cnx != NULL);
   cnx_list.list = g_list_prepend(cnx_list.list, cnx);
 }
 
@@ -104,7 +105,7 @@
   O_MUTEX_INIT(cnx->outgoing_mutex);
 }
 
-static void
+void
 giop_connection_close (GIOPConnection *cnx)
 {
   if(cnx->parent.status == LINC_DISCONNECTED)
@@ -125,7 +126,7 @@
 }
 
 static void
-giop_connection_shutdown    (GObject             *obj)
+giop_connection_shutdown (GObject *obj)
 {
   GIOPConnection *cnx = (GIOPConnection *)obj;
 
@@ -133,6 +134,10 @@
   O_MUTEX_DESTROY(cnx->incoming_mutex);
   O_MUTEX_DESTROY(cnx->outgoing_mutex);
 
+  O_MUTEX_LOCK(cnx_list.lock);
+  giop_connection_list_remove(cnx);
+  O_MUTEX_UNLOCK(cnx_list.lock);
+
   if(parent_class->shutdown)
     parent_class->shutdown(obj);
 }
@@ -265,11 +270,7 @@
   if(!cnx)
     return;
 
-  O_MUTEX_LOCK(cnx_list.lock);
-  if(G_OBJECT(cnx)->ref_count == 1)
-    giop_connection_list_remove(cnx);
   g_object_unref(G_OBJECT(cnx));
-  O_MUTEX_UNLOCK(cnx_list.lock);
 }
 
 /* This will just create the fd, do the connect and all, and then call
@@ -299,6 +300,8 @@
 	}
       else
 	giop_connection_list_add(cnx);
+
+      g_object_ref (G_OBJECT(cnx));
     }
 
   O_MUTEX_UNLOCK(cnx_list.lock);
@@ -309,16 +312,18 @@
 void
 giop_connection_remove_by_orb(gpointer match_orb_data)
 {
-  GList *link;
+  GList *l, *next;
 
   O_MUTEX_LOCK(cnx_list.lock);
-  for (link = cnx_list.list; link; )
+  for (l = cnx_list.list; l; l = next)
     {
-      GIOPConnection *cnx = link->data;
-      link = link->next;
-      if ( cnx->orb_data == match_orb_data )
-	cnx_list.list = g_list_delete_link(cnx_list.list, link);
-      giop_connection_close (cnx);
+      GIOPConnection *cnx = l->data;
+
+      next = l->next;
+      if ( cnx->orb_data == match_orb_data ) {
+	cnx_list.list = g_list_delete_link(cnx_list.list, l);
+	giop_connection_close (cnx);
+      }
     }
   O_MUTEX_UNLOCK(cnx_list.lock);
 }
Index: src/orb/orb-core/corba-context.c
===================================================================
RCS file: /cvs/gnome/ORBit2/src/orb/orb-core/corba-context.c,v
retrieving revision 1.5.2.3
diff -u -r1.5.2.3 corba-context.c
--- src/orb/orb-core/corba-context.c	2001/05/18 18:45:30	1.5.2.3
+++ src/orb/orb-core/corba-context.c	2001/05/24 20:36:43
@@ -98,6 +98,7 @@
     ctx->mappings = g_hash_table_new(g_str_hash, g_str_equal);
 
   if(g_hash_table_lookup_extended(ctx->mappings, prop_name, &old_nom, &old_value)) {
+    g_hash_table_remove (ctx->mappings, prop_name);
     g_free(old_nom);
     g_free(old_value);
   }
Index: src/orb/orb-core/corba-object.c
===================================================================
RCS file: /cvs/gnome/ORBit2/src/orb/orb-core/corba-object.c,v
retrieving revision 1.19.2.9
diff -u -r1.19.2.9 corba-object.c
--- src/orb/orb-core/corba-object.c	2001/05/24 11:58:11	1.19.2.9
+++ src/orb/orb-core/corba-object.c	2001/05/24 20:36:43
@@ -60,7 +60,8 @@
   g_hash_table_remove (objrefs, obj);
 
   if (obj->connection) {
-/*    g_warning("Release object '%p's connection", obj);*/
+/*    g_warning("Release object '%p's connection", obj); */
+    giop_connection_close (obj->connection);
     giop_connection_unref (obj->connection);
   }
 
@@ -303,7 +304,6 @@
       if(IOP_profile_get_info(obj, pi, &iiop_version, &proto,
 			      &host, &service, &is_ssl, &oki, tbuf))
 	{
-          g_warning ("Initiated new connection on '%s'", obj->type_id);
 	  obj->connection =
 	    giop_connection_initiate(proto, host, service,
 				     is_ssl?LINC_CONNECTION_SSL:0, iiop_version);
Index: test/everything/Makefile.am
===================================================================
RCS file: /cvs/gnome/ORBit2/test/everything/Makefile.am,v
retrieving revision 1.2.2.15
diff -u -r1.2.2.15 Makefile.am
--- test/everything/Makefile.am	2001/05/24 07:38:19	1.2.2.15
+++ test/everything/Makefile.am	2001/05/24 20:36:43
@@ -19,14 +19,14 @@
 
 LDADD = $(top_builddir)/src/orb/libORBit-2.la		\
 	$(LINC_LIBS)					\
-	${GLIB_LIBS} 
+	${GLIB_LIBS}
+#  -lefence
 
 LDFLAGS = -static
 
 client_SOURCES=${EVERYTHING_BUILT} client.c everything.idl constants.h
 client_DEPENDENCES=${EVERYTHING_BUILT} $(included_src)
 client_LDADD = $(LDADD)
-# -lefence
 
 included_src = \
 	basicServer.c    \

? elliot
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/linc/ChangeLog,v
retrieving revision 1.6
diff -u -r1.6 ChangeLog
--- ChangeLog	2001/05/24 09:57:48	1.6
+++ ChangeLog	2001/05/24 20:36:30
@@ -1,4 +1,16 @@
+2001-05-25  Michael Meeks  <michael ximian com>
+
+	* src/linc-connection.c (linc_source_remove): split from
+	(linc_connection_shutdown): here, and release the self
+	reference whilst watching.
+	(linc_connection_real_state_changed): use linc_source_remove.
+	(linc_connection_connected): hold a ref over the state_changed
+	emissions.
+
 2001-05-24  Michael Meeks  <michael ximian com>
+
+	* src/linc-connection.c (linc_connection_connected):
+	unref the connection object on disconnection.
 
 	* src/linc-server.c (linc_server_destroy):
 	protect against multiple shutdowns, and rename to
Index: src/linc-connection.c
===================================================================
RCS file: /cvs/gnome/linc/src/linc-connection.c,v
retrieving revision 1.20
diff -u -r1.20 linc-connection.c
--- src/linc-connection.c	2001/05/24 09:57:49	1.20
+++ src/linc-connection.c	2001/05/24 20:36:30
@@ -61,13 +61,22 @@
 }
 
 static void
-linc_connection_shutdown    (GObject             *obj)
+linc_source_remove (LINCConnection *cnx, gboolean unref)
 {
-  LINCConnection *cnx = (LINCConnection *)obj;
-
-  if(cnx->tag)
+  if(cnx->tag) {
     g_assert (g_source_remove(cnx->tag));
+    if (unref) 
+      g_object_unref (G_OBJECT (cnx));
+  }
   cnx->tag = 0;
+}
+
+static void
+linc_connection_shutdown (GObject *obj)
+{
+  LINCConnection *cnx = (LINCConnection *)obj;
+
+  linc_source_remove (cnx, FALSE);
   g_free(cnx->remote_host_info);
   cnx->remote_host_info = NULL;
   g_free(cnx->remote_serv_info);
@@ -89,6 +98,9 @@
   int rv, n;
   socklen_t n_size = sizeof(n);
   gboolean retval = TRUE;
+  gboolean disconnected = FALSE;
+
+  g_object_ref (G_OBJECT (cnx));
 
   switch(cnx->status)
     {
@@ -104,16 +116,22 @@
 				    linc_connection_connected, cnx);
 	  retval = FALSE;
 	}
-      else
+      else {
 	linc_connection_state_changed(cnx, LINC_DISCONNECTED);
+	disconnected = TRUE;
+      }
       break;
-    case LINC_CONNECTED:
+    case LINC_CONNECTED: {
       linc_connection_state_changed(cnx, LINC_DISCONNECTED);
+      disconnected = TRUE;
       break;
+    }
     default:
       break;
     }
 
+  g_object_unref (G_OBJECT (cnx));
+
   return retval;
 }
 
@@ -138,15 +156,15 @@
 				  linc_connection_connected, cnx);
       break;
     case LINC_CONNECTING:
-      if(cnx->tag)
-	g_assert (g_source_remove(cnx->tag));
+      linc_source_remove (cnx, FALSE);
       cnx->tag = g_io_add_watch(cnx->gioc, G_IO_OUT|G_IO_ERR|G_IO_HUP|G_IO_NVAL, linc_connection_connected, cnx);
       break;
     case LINC_DISCONNECTED:
-      if(cnx->tag)
-	g_assert (g_source_remove(cnx->tag));
-      cnx->tag = 0;
-      close(cnx->fd);
+/*      g_warning ("Linc disconnected tag %d fd '%d'",
+	cnx->tag, cnx->fd);*/
+      linc_source_remove (cnx, TRUE);
+      if (cnx->fd >= 0)
+        close(cnx->fd);
       cnx->fd = -1;
       break;
     }
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]