ORBit - typecode ref counting bug ...

[Michael Meeks <michael ximian com>]

Hi Guys,

	This fixes a very nasty bug with Anys - passing sequences of
anything would multiply release the typecode and cause extreme badness, it
also seems the flow control was crack smoking.

	So ... this will only work with an integrated bonobo fix - to
come shortly, and fixes several registered bugs.

	If anyone can see anything stupid; please shout now.

	Regards,

		Michael.

? ORBit-0.5.11.tar.gz
? ORBit-0.5.10.tar.gz
? a.out
? elliot
? src/orbit-idl-compiler/gstmediaplay.gst
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/ORBit/ChangeLog,v
retrieving revision 1.11.4.6
diff -u -r1.11.4.6 ChangeLog
--- ChangeLog	2001/10/23 00:24:24	1.11.4.6
+++ ChangeLog	2001/10/31 10:21:12
@@ -1,3 +1,20 @@
+2001-11-01  Michael Meeks  <michael@ximian.com>
+
+	* src/orb/allocators.c (ORBit_free): fix hideous flow
+	control and, release the typecode once here.
+	(ORBit_free_via_TypeCode): don't release the typecode
+	here - we might be in a sequence or somesuch. Remove
+	slow bloat of continualy re-referencing types. Make the
+	code look pleasant.
+
+	* test/everything/client.c (testAnyStrSeq): impl.
+	(main): hook it up.
+
+	* test/everything/anyServer.c (AnyServer_opAnyStrSeq): impl.
+
+	* src/orb/orbit_typecode.c (ORBit_TypeCode_release):
+	zap the memory so we can detect badness more easily.
+
 2001-10-24  Michael Meeks  <michael@ximian.com>

 	* Version 0.5.11
Index: src/orb/allocators.c
===================================================================
RCS file: /cvs/gnome/ORBit/src/orb/allocators.c,v
retrieving revision 1.34.4.6
diff -u -r1.34.4.6 allocators.c
--- src/orb/allocators.c	2001/10/01 14:21:38	1.34.4.6
+++ src/orb/allocators.c	2001/10/31 10:21:14
@@ -108,10 +108,10 @@
 {
 	ORBit_mem_info *block;

-	if(!mem)
+	if (!mem)
 		return;

-	block = PTR_TO_MEMINFO(mem);
+	block = PTR_TO_MEMINFO (mem);

 	/* This block has not been allocated by CORBA_alloc. Instead
          * it is memory in the receive buffer, that has been used in
@@ -120,132 +120,123 @@
 		return;

 #ifdef ORBIT_DEBUG
-	g_assert(block->magic == 0xdeadbeef);
+	g_assert (block->magic == 0xdeadbeef);
 #endif

-	if(block->free) {
-		int i;
+	if (block->free) {
+		int      i;
 		gpointer x;
 		gpointer my_data;

-		if((gpointer)block->free == (gpointer)ORBit_free_via_TypeCode)
-			my_data = ((guchar *)block) - sizeof(CORBA_TypeCode);
+		if ((gpointer) block->free == (gpointer) ORBit_free_via_TypeCode)
+			my_data = ((guchar *) block) - sizeof(CORBA_TypeCode);
 		else
 			my_data = NULL;

 #ifdef ORBIT_DEBUG
-		if(block->func_data == NULL)
+		if (block->func_data == NULL)
 			g_warning("block with freefunc %p has no items", block->free);
 #endif

-		for(i = 0, x = mem; i < (gulong)block->func_data; i++)
+		for (i = 0, x = mem; i < (gulong) block->func_data; i++)
 			x = block->free(x, my_data, CORBA_TRUE);

-		if((gpointer)block->free == (gpointer)ORBit_free_via_TypeCode)
-			/* ((guchar *)block) -= sizeof(CORBA_TypeCode); */
-			block = (ORBit_mem_info *)
-				(((guchar *)block) - sizeof(CORBA_TypeCode));
-		g_free(block);
-	} else
-		g_free(block);
+		if (my_data) {
+			CORBA_Object_release (*(CORBA_Object *)my_data, NULL);
+			block = my_data;
+		}
+	}
+	g_free (block);
 }

-/******************************************************************/
-/* These aren't currently used... */
-
 gpointer
-ORBit_free_via_TypeCode(gpointer mem, gpointer tcp, gboolean ignore)
+ORBit_free_via_TypeCode (gpointer mem, gpointer tcp, gboolean ignore)
 {
-	CORBA_TypeCode tc = *(CORBA_TypeCode *)tcp, subtc;
 	int i;
 	guchar *retval = NULL;
+	CORBA_TypeCode tc = *(CORBA_TypeCode *) tcp;

 	switch(tc->kind) {
-	case CORBA_tk_any:
-		{
-			CORBA_any *anyval = mem;
-			if(anyval->_release)
-				CORBA_free(anyval->_value);
-			retval = (guchar *)(anyval + 1);
-		}
+	case CORBA_tk_any: {
+		CORBA_any *anyval = mem;
+		if (anyval->_release)
+			CORBA_free (anyval->_value);
+		retval = (guchar *)(anyval + 1);
 		break;
+	}
 	case CORBA_tk_TypeCode:
 	case CORBA_tk_objref:
-		{
-			CORBA_Object_release(*(CORBA_Object *)mem, NULL);
+		CORBA_Object_release (*(CORBA_Object *) mem, NULL);

-			retval = (guchar *)mem + sizeof(CORBA_Object);
-		}
+		retval = (guchar *) mem + sizeof(CORBA_Object);
 		break;
-	case CORBA_tk_Principal:
-		{
-			CORBA_Principal *pval = mem;
-			if(pval->_release)
-				CORBA_free(pval->_buffer);
-			retval = (guchar *)(pval + 1);
-		}
+	case CORBA_tk_Principal: {
+		CORBA_Principal *pval = mem;
+
+		if (pval->_release)
+			CORBA_free (pval->_buffer);
+
+		retval = (guchar *)(pval + 1);
 		break;
+	}
 	case CORBA_tk_except:
 	case CORBA_tk_struct:
-		mem = ALIGN_ADDRESS(mem, ORBit_find_alignment(tc));
-		for(i = 0; i < tc->sub_parts; i++) {
-			subtc = (CORBA_TypeCode)CORBA_Object_duplicate((CORBA_Object)tc->subtypes[i], NULL);
-			mem = ORBit_free_via_TypeCode(mem, &subtc,
-						      CORBA_TRUE);
-		}
+		mem = ALIGN_ADDRESS (mem, ORBit_find_alignment (tc));
+		for (i = 0; i < tc->sub_parts; i++)
+			mem = ORBit_free_via_TypeCode (
+				mem, &tc->subtypes[i], CORBA_TRUE);
 		retval = mem;
 		break;
-	case CORBA_tk_union:
-		subtc = (CORBA_TypeCode)CORBA_Object_duplicate(
-		  (CORBA_Object)ORBit_get_union_tag(tc, &mem, TRUE), NULL);
-		{
-		    int sz = 0;
-		    int al = 1;
-		    for(i = 0; i < tc->sub_parts; i++) {
-		        al = MAX(al, ORBit_find_alignment(tc->subtypes[i]));
-		        sz = MAX(sz, ORBit_gather_alloc_info(tc->subtypes[i]));
-		    }
-		    mem = ALIGN_ADDRESS(mem, al);
-		    ORBit_free_via_TypeCode(mem, &subtc, CORBA_TRUE);
-		    /* the end of the body (subtc) may not be the
-		     * same as the end of the union */
-		    retval = (guchar *)mem + sz;
-		}
+	case CORBA_tk_union: {
+		CORBA_TypeCode tag;
+		int sz = 0;
+		int al = 1;
+
+		tag = ORBit_get_union_tag (tc, &mem, TRUE);
+
+		for (i = 0; i < tc->sub_parts; i++) {
+			al = MAX (al, ORBit_find_alignment (tc->subtypes[i]));
+			sz = MAX (sz, ORBit_gather_alloc_info (tc->subtypes[i]));
+		}
+		mem = ALIGN_ADDRESS (mem, al);
+		ORBit_free_via_TypeCode (mem, &tag, CORBA_TRUE);
+		/* the end of the body (subtc) may not be the
+		 * same as the end of the union */
+		retval = (guchar *)mem + sz;
 		break;
+	}
 	case CORBA_tk_wstring:
 	case CORBA_tk_string:
-			CORBA_free(*(char **)mem);
-		retval = (guchar *)mem + sizeof(char *);
+		CORBA_free (*(char **) mem);
+		retval = (guchar *) mem + sizeof (char *);
 		break;
-	case CORBA_tk_sequence:
-		{
-			CORBA_sequence_octet *pval = mem;
-			if(pval->_release)
-				CORBA_free(pval->_buffer);
+	case CORBA_tk_sequence: {
+		CORBA_sequence_octet *pval = mem;

-			retval = (guchar *)mem + sizeof(CORBA_sequence_octet);
-		}
+		if (pval->_release)
+			CORBA_free (pval->_buffer);
+
+		retval = (guchar *)mem + sizeof (CORBA_sequence_octet);
 		break;
+	}
 	case CORBA_tk_array:
-		for(i = 0; i < tc->length; i++) {
-			subtc = (CORBA_TypeCode)CORBA_Object_duplicate((CORBA_Object)tc->subtypes[0], NULL);
-			mem = ORBit_free_via_TypeCode(mem, &subtc,
-						      CORBA_TRUE);
-		}
+		for (i = 0; i < tc->length; i++)
+			mem = ORBit_free_via_TypeCode(
+				mem, &tc->subtypes[0], CORBA_TRUE);
 		retval = mem;
 		break;
 	case CORBA_tk_alias:
-		subtc = (CORBA_TypeCode)CORBA_Object_duplicate((CORBA_Object)tc->subtypes[0], NULL);
-		retval = ORBit_free_via_TypeCode(mem, &subtc, CORBA_TRUE);
+		retval = ORBit_free_via_TypeCode (mem, &tc->subtypes[0], CORBA_TRUE);
 		break;
-	default:
-		/* NB. alignment != alloc size */
-		retval = (guchar *)ALIGN_ADDRESS (mem, ORBit_find_alignment (tc)) +
-			ORBit_gather_alloc_info (tc);
+	default: {
+		int sz = ORBit_gather_alloc_info (tc);
+		int al = ORBit_find_alignment (tc);
+
+		/* NB. often alignment != alloc size */
+		retval = (guchar *) ALIGN_ADDRESS (mem, al) + sz;
 		break;
 	}
-
-	CORBA_Object_release((CORBA_Object)tc, NULL);
+	}

-	return (gpointer)retval;
+	return (gpointer) retval;
 }
Index: src/orb/orbit_typecode.c
===================================================================
RCS file: /cvs/gnome/ORBit/src/orb/orbit_typecode.c,v
retrieving revision 1.24.4.4
diff -u -r1.24.4.4 orbit_typecode.c
--- src/orb/orbit_typecode.c	2001/03/09 10:25:48	1.24.4.4
+++ src/orb/orbit_typecode.c	2001/10/31 10:21:16
@@ -228,7 +228,7 @@
 }

 static void
-ORBit_TypeCode_release(gpointer obj, CORBA_Environment *ev)
+ORBit_TypeCode_release (gpointer obj, CORBA_Environment *ev)
 {
   /* we will initialize the TC_ constants with a negative refcount */
   if(ORBIT_ROOT_OBJECT(obj)->refs >= 0) {
@@ -258,6 +258,8 @@

       if(tc->discriminator)
 	CORBA_Object_release((CORBA_Object)tc->discriminator, ev);
+
+      memset (obj, 0xa, sizeof (struct CORBA_TypeCode_struct));

       g_free(obj);
     }
Index: test/everything/anyServer.c
===================================================================
RCS file: /cvs/gnome/ORBit/test/everything/Attic/anyServer.c,v
retrieving revision 1.1.2.4
diff -u -r1.1.2.4 anyServer.c
--- test/everything/anyServer.c	2001/03/09 09:27:18	1.1.2.4
+++ test/everything/anyServer.c	2001/10/31 10:21:16
@@ -31,6 +31,32 @@

 static
 CORBA_any *
+AnyServer_opAnyStrSeq (PortableServer_Servant _servant,
+		       CORBA_Environment * ev)
+{
+	CORBA_any   *retn;
+	test_StrSeq *seq;
+	int          i;
+
+	seq = test_StrSeq__alloc();
+	seq->_length = 16;
+	seq->_buffer = CORBA_sequence_CORBA_string_allocbuf (seq->_length);
+	seq->_release = TRUE;
+
+	for (i = 0; i < seq->_length; i++)
+		seq->_buffer [i] = CORBA_string_dup ("Foo");
+
+	retn = CORBA_any_alloc ();
+	retn->_type = (CORBA_TypeCode) CORBA_Object_duplicate (
+		(CORBA_Object) TC_test_StrSeq, ev);
+	retn->_value = seq;
+	retn->_release = TRUE;
+
+	return retn;
+}
+
+static
+CORBA_any *
 AnyServer_opAnyLong(PortableServer_Servant _servant,
 					const CORBA_any * inArg,
 					CORBA_any * inoutArg,
@@ -160,8 +186,6 @@
 					 CORBA_TypeCode * inoutArg,
 					 CORBA_TypeCode * outArg,
 					 CORBA_Environment * ev){
-  CORBA_TypeCode retn;
-
   g_assert(CORBA_TypeCode_equal(inArg,TC_test_ArrayUnion,ev) );
   g_assert(CORBA_TypeCode_equal(*inoutArg,TC_test_AnyServer,ev) );

@@ -175,11 +199,12 @@
 PortableServer_ServantBase__epv AnyServer_base_epv = {NULL,NULL,NULL};

 POA_test_AnyServer__epv AnyServer_epv = {
-  NULL,
-  AnyServer_opAnyLong,
-  AnyServer_opAnyString,
-  AnyServer_opAnyStruct,
-  AnyServer_opTypeCode,
+	NULL,
+	AnyServer_opAnyStrSeq,
+	AnyServer_opAnyLong,
+	AnyServer_opAnyString,
+	AnyServer_opAnyStruct,
+	AnyServer_opTypeCode,
 };

 POA_test_AnyServer__vepv AnyServer_vepv = {&AnyServer_base_epv,&AnyServer_epv};
Index: test/everything/client.c
===================================================================
RCS file: /cvs/gnome/ORBit/test/everything/Attic/client.c,v
retrieving revision 1.1.2.9
diff -u -r1.1.2.9 client.c
--- test/everything/client.c	2001/10/22 09:43:01	1.1.2.9
+++ test/everything/client.c	2001/10/31 10:21:18
@@ -575,6 +575,26 @@
   g_assert(ev->_major == CORBA_NO_EXCEPTION);
 }

+void
+testAnyStrSeq (test_TestFactory   factory,
+	       CORBA_Environment *ev)
+{
+	test_AnyServer objref;
+	CORBA_any     *retn;
+
+	d_print ("Testing any with string sequences ...\n");
+
+	objref = test_TestFactory_getAnyServer(factory,ev);
+	g_assert(ev->_major == CORBA_NO_EXCEPTION);
+
+	retn = test_AnyServer_opAnyStrSeq (objref, ev);
+	g_assert(ev->_major == CORBA_NO_EXCEPTION);
+
+	CORBA_free (retn);
+
+	CORBA_Object_release (objref, ev);
+}
+
 void testAnyStruct(test_TestFactory factory,
 				   CORBA_Environment *ev) {
   test_AnyServer objref;
@@ -747,6 +767,7 @@
   testAnyLong(factory,&ev);
   testAnyString(factory,&ev);
   testAnyStruct(factory,&ev);
+  testAnyStrSeq(factory,&ev);
   testAnyException(factory,&ev);
   testSequenceOfAny(factory,&ev);
   testTypeCode(factory,&ev);
Index: test/everything/everything.idl
===================================================================
RCS file: /cvs/gnome/ORBit/test/everything/Attic/everything.idl,v
retrieving revision 1.1.2.3
diff -u -r1.1.2.3 everything.idl
--- test/everything/everything.idl	2001/04/04 13:57:17	1.1.2.3
+++ test/everything/everything.idl	2001/10/31 10:21:18
@@ -202,6 +202,7 @@
 	};

 	interface AnyServer {
+		any opAnyStrSeq();
 		any opAnyLong(in any inArg, inout any inoutArg, out any outArg);
 		any opAnyString(in any inArg, inout any inoutArg, out any outArg);
 		any opAnyStruct(in any inArg, inout any inoutArg, out any outArg);

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