Memory leak fixes for ffcall with exceptions being passed from server to

client.


git-svn-id: svn+ssh://svn.gna.org/svn/gnustep/libs/base/trunk@10945 72102866-910b-0410-8b05-ffd578937521

ChangeLog

@@ -1,3 +1,17 @@
+2001-09-20  Richard Frith-Macdonald <rfm@gnu.org>
+
+        * Headers/gnustep/base/DistributedObjects.h:
+        * Source/GSFFCallInvocation.m:
+        * Source/NSConnection.m:
+        * Source/NSInvocation.m:
+        * Source/callframe.h:
+        * Source/callframe.m:
+	Modifications to callframe handling to store pointers to data to
+	be freed in order to be able to tidy up after an exception ...
+	Appears to cure memory leak in ffcall code on server side.
+	No fix yest for mframe or ffi code, or for client side leak
+	when an exception occurs in the server.
+
 2001-09-20  Richard Frith-Macdonald <rfm@gnu.org>
 
 	* Testing/nsconnection_client.m: Added simple exception tests

Headers/gnustep/base/DistributedObjects.h

@@ -86,6 +86,12 @@ typedef struct {
   NSPortCoder	*decoder;	// The coder to use
   NSPortCoder	*encoder;	// The coder to use
   unsigned	seq;		// Sequence number
+  /*
+   * These next fields can store allocated memory that will need to be
+   * tidied up iff an exception occurs before they can be tidied normally.
+   */
+  void		*datToFree;	// Data needing NSZoneFree()
+  id		objToFree;	// Data needing NSDeallocateObject()
 } DOContext;
 
 #endif /* __DistributedObjects_h */

Source/GSFFCallInvocation.m

@@ -464,7 +464,7 @@ static IMP gs_objc_msg_forward (SEL sel)
 }
 
 /*
- *	This is the de-signated initialiser.
+ *	This is the designated initialiser.
  */
 - (id) initWithMethodSignature: (NSMethodSignature*)aSignature
 {
@@ -472,10 +472,6 @@ static IMP gs_objc_msg_forward (SEL sel)
   _numArgs = [aSignature numberOfArguments];
   _info = [aSignature methodInfo];
   _cframe = callframe_from_info(_info, _numArgs, &_retval);
-  if (_retval == 0 && _info[0].size > 0)
-    {
-      _retval = NSZoneMalloc(NSDefaultMallocZone(), _info[0].size);
-    }
   return self;
 }
 

Source/NSConnection.m

@@ -1541,13 +1541,30 @@ static void retDecoder(DOContext *ctxt)
 	  [coder decodeValueOfObjCType: @encode(id) at: &exc];
 	  ctxt->decoder = nil;
 	  [ctxt->connection _doneInRmc: coder];
+	  if (ctxt->datToFree != 0)
+	    {
+	      NSZoneFree(NSDefaultMallocZone(), ctxt->datToFree);
+	      ctxt->datToFree = 0;
+	    }
+	  if (ctxt->objToFree != nil)
+	    {
+	      NSDeallocateObject(ctxt->objToFree);
+	      ctxt->objToFree = nil;
+	    }
 	  [exc raise];
 	}
     }
-  [coder decodeValueOfObjCType: type at: ctxt->datum];
   if (*type == _C_ID)
     {
-      AUTORELEASE(*(id*)ctxt->datum);
+      *(id*)ctxt->datum = [coder decodeObject];
+    }
+  else
+    {
+      [coder decodeValueOfObjCType: type at: ctxt->datum];
+      if ((*type == _C_CHARPTR || *type == _C_PTR) && *(void**)ctxt->datum != 0)
+	{
+	  [NSData dataWithBytesNoCopy: *(void**)ctxt->datum length: 1];
+	}
     }
 }
 
@@ -1557,13 +1574,19 @@ static void retEncoder (DOContext *ctxt)
     {
     case _C_ID: 
       if (ctxt->flags & _F_BYCOPY)
-	[ctxt->encoder encodeBycopyObject: *(id*)ctxt->datum];
+	{
+	  [ctxt->encoder encodeBycopyObject: *(id*)ctxt->datum];
+	}
 #ifdef	_F_BYREF
       else if (ctxt->flags & _F_BYREF)
-	[ctxt->encoder encodeByrefObject: *(id*)ctxt->datum];
+	{
+	  [ctxt->encoder encodeByrefObject: *(id*)ctxt->datum];
+	}
 #endif
       else
-	[ctxt->encoder encodeObject: *(id*)ctxt->datum];
+	{
+	  [ctxt->encoder encodeObject: *(id*)ctxt->datum];
+	}
       break;
     default: 
       [ctxt->encoder encodeValueOfObjCType: ctxt->type at: ctxt->datum];
@@ -2050,8 +2073,6 @@ static void retEncoder (DOContext *ctxt)
 static void callDecoder (DOContext *ctxt)
 {
   const char	*type = ctxt->type;
-  void		*datum = ctxt->datum;
-  NSPortCoder	*coder = ctxt->decoder;
 
   /*
    * We need this "dismiss" to happen here and not later so that Coder
@@ -2059,37 +2080,45 @@ static void callDecoder (DOContext *ctxt)
    * objects is invoked.  We clear the 'decoder' field in the context to
    * show that it is no longer valid.
    */
-  if (datum == 0 && type == 0)
+  if (type == 0)
     {
+      NSPortCoder	*coder = ctxt->decoder;
+
       ctxt->decoder = nil;
       [ctxt->connection _doneInRmc: coder];
       return;
     }
 
-  [coder decodeValueOfObjCType: type at: datum];
+  /*
-#ifdef USE_FFCALL
+   * The coder may have an optimised method for decoding objects
+   * so we use that one if we are expecting an object, otherwise
+   * we use thegeneric method.
+   */
   if (*type == _C_ID)
-#else
-  /* -decodeValueOfObjCType: at: malloc's new memory
-     for char*'s.  We need to make sure it gets freed eventually
-     so we don't have a memory leak.  Request here that it be
-     autorelease'ed. Also autorelease created objects. */
-  if ((*type == _C_CHARPTR || *type == _C_PTR) && *(void**)datum != 0)
     {
-      [NSData dataWithBytesNoCopy: *(void**)datum length: 1];
+      *(id*)ctxt->datum = [ctxt->decoder decodeObject];
     }
-  else if (*type == _C_ID)
+  else
-#endif
     {
-      AUTORELEASE(*(id*)datum);
+      void	*datum = ctxt->datum;
+
+      [ctxt->decoder decodeValueOfObjCType: type at: datum];
+      /*
+       * -decodeValueOfObjCType:at: malloc's new memory
+       * for pointers.  We need to make sure it gets freed eventually
+       * so we don't have a memory leak.  Request here that it be
+       * autorelease'ed.
+       */
+      if ((*type == _C_CHARPTR || *type == _C_PTR) && *(void**)datum != 0)
+	{
+	  [NSData dataWithBytesNoCopy: *(void**)datum length: 1];
+	}
     }
 }
 
 static void callEncoder (DOContext *ctxt)
 {
   const char		*type = ctxt->type;
-  void			*datum = ctxt->datum;
-  int			flags = ctxt->flags;
   NSPortCoder		*coder = ctxt->encoder;
 
   if (coder == nil)
@@ -2111,27 +2140,35 @@ static void callEncoder (DOContext *ctxt)
        * later use if/when we are called again.  We encode a flag to
        * say that this is not an exception.
        */
-      coder = [ctxt->connection _makeOutRmc: ctxt->seq
+      ctxt->encoder = [ctxt->connection _makeOutRmc: ctxt->seq
-				   generate: 0
+					   generate: 0
-				      reply: NO];
+					      reply: NO];
-      ctxt->encoder = coder;
+      coder = ctxt->encoder;
       [coder encodeValueOfObjCType: @encode(BOOL) at: &is_exception];
     }
 
-  switch (*type)
+  if (*type == _C_ID)
     {
-      case _C_ID: 
+      int	flags = ctxt->flags;
-	if (flags & _F_BYCOPY)
+
-	  [coder encodeBycopyObject: *(id*)datum];
+      if (flags & _F_BYCOPY)
+	{
+	  [coder encodeBycopyObject: *(id*)ctxt->datum];
+	}
 #ifdef	_F_BYREF
-	else if (flags & _F_BYREF)
+      else if (flags & _F_BYREF)
-	  [coder encodeByrefObject: *(id*)datum];
+	{
+	  [coder encodeByrefObject: *(id*)ctxt->datum];
+	}
 #endif
-	else
+      else
-	  [coder encodeObject: *(id*)datum];
+	{
-	break;
+	  [coder encodeObject: *(id*)ctxt->datum];
-      default: 
+	}
-	[coder encodeValueOfObjCType: type at: datum];
+    }
+  else
+    {
+      [coder encodeValueOfObjCType: type at: ctxt->datum];
     }
 }
 
@@ -2196,6 +2233,16 @@ static void callEncoder (DOContext *ctxt)
 	    {
 	      NSPortCoder	*op;
 
+	      if (ctxt.datToFree != 0)
+		{
+		  NSZoneFree(NSDefaultMallocZone(), ctxt.datToFree);
+		  ctxt.datToFree = 0;
+		}
+	      if (ctxt.objToFree != nil)
+		{
+		  NSDeallocateObject(ctxt.objToFree);
+		  ctxt.objToFree = nil;
+		}
 	      if (ctxt.decoder != nil)
 		{
 		  [self _failInRmc: ctxt.decoder];

Source/NSInvocation.m

@@ -169,7 +169,10 @@ _arg_addr(NSInvocation *inv, int index)
 #else
 #ifdef USE_FFCALL
   if (_cframe)
-    callframe_free((callframe_t *)_cframe);
+    {
+      NSZoneFree(NSDefaultMallocZone(), _cframe);
+      _retval = 0;	// Part of _cframe
+    }
 #endif
 #endif
   if (_argframe)

Source/callframe.h

@@ -38,7 +38,6 @@ typedef struct _callframe_t {
 
 extern callframe_t *callframe_from_info (NSArgumentInfo *info, int numargs,
 					 void **retval);
-extern void callframe_free(callframe_t *cframe);
 extern void callframe_set_arg(callframe_t *cframe, int index, void *buffer,
 			      int size);
 extern void callframe_get_arg(callframe_t *cframe, int index, void *buffer,

Source/callframe.m

@@ -75,7 +75,33 @@ callframe_from_info (NSArgumentInfo *info, int numargs, void **retval)
         }
     }
 
-  cframe = buf = malloc(size);
+  /*
+   * If we need space allocated to store a return value,
+   * make room for it at the end of the callframe so we
+   * only need to do a single malloc.
+   */
+  if (retval)
+    {
+      unsigned	full = size;
+      unsigned	pos;
+
+      if (full % align != 0)
+	{
+	  full += (align - full % align);
+	}
+      pos = full;
+      full += MAX(info[0].size, sizeof(smallret_t));
+      cframe = buf = NSZoneMalloc(NSDefaultMallocZone(), full);
+      if (cframe)
+	{
+	  *retval = buf + pos;
+	}
+    }
+  else
+    {
+      cframe = buf = NSZoneMalloc(NSDefaultMallocZone(), size);
+    }
+
   if (cframe)
     {
       cframe->nargs = numargs;
@@ -98,20 +124,9 @@ callframe_from_info (NSArgumentInfo *info, int numargs, void **retval)
         }
     }
 
-  if (retval)
-    {
-      *retval = NSZoneMalloc(NSDefaultMallocZone(),
-                            MAX(info[0].size, sizeof(smallret_t)) );
-    }
   return cframe;
 }
 
-void
-callframe_free(callframe_t *cframe)
-{
-  free(cframe);
-}
-
 void
 callframe_set_arg(callframe_t *cframe, int index, void *buffer, int size)
 {
@@ -232,8 +247,6 @@ callframe_do_call (DOContext *ctxt,
   unsigned flags;
   /* Which argument number are we processing now? */
   int argnum;
-  /* A pointer to the memory holding the return value of the method. */
-  void *retval;
   /* The cif information for calling the method */
   callframe_t *cframe;
   /* Does the method have any arguments that are passed by reference?
@@ -243,6 +256,7 @@ callframe_do_call (DOContext *ctxt,
   NSInvocation_t *inv;
   /* Signature information */
   NSMethodSignature *sig;
+  void	*retval;
   const char *encoded_types = ctxt->type;
 
   /* Decode the object, (which is always the first argument to a method),
@@ -297,6 +311,7 @@ callframe_do_call (DOContext *ctxt,
   sig = [NSMethodSignature signatureWithObjCTypes: type];
   cframe = callframe_from_info([sig methodInfo], [sig numberOfArguments], 
 			       &retval);
+  ctxt->datToFree = cframe;
 
   /* Put OBJECT and SELECTOR into the ARGFRAME. */
 
@@ -428,8 +443,10 @@ callframe_do_call (DOContext *ctxt,
   inv->_cframe = cframe;
   inv->_info = [sig methodInfo];
   inv->_numArgs = [sig numberOfArguments];
+  ctxt->objToFree = (id)inv;
   GSFFCallInvokeWithTargetAndImp((NSInvocation *)inv, object, 
 				 method_implementation);
+  ctxt->objToFree = nil;
   NSDeallocateObject((NSInvocation *)inv);
 
   /* Encode the return value and pass-by-reference values, if there
@@ -536,9 +553,8 @@ callframe_do_call (DOContext *ctxt,
 	}
     }
 
-  if (retval != 0)
+  NSZoneFree(NSDefaultMallocZone(), ctxt->datToFree);
-    NSZoneFree(NSDefaultMallocZone(), retval);
+  ctxt->datToFree = 0;
-  callframe_free(cframe);
 
   return;
 }
@@ -580,6 +596,7 @@ callframe_build_return (NSInvocation *inv,
   sig = [NSMethodSignature signatureWithObjCTypes: type];
   cframe = callframe_from_info([sig methodInfo], [sig numberOfArguments], 
 			       &retval);
+  ctxt->datToFree = cframe;
 
   /* Get the return type qualifier flags, and the return type. */
   flags = objc_get_type_qualifiers(type);
@@ -722,9 +739,11 @@ callframe_build_return (NSInvocation *inv,
       (*decoder) (ctxt);	/* Tell it we have finished.	*/
     }
 
-  if (retval != 0)
+  if (ctxt->datToFree != 0)
-    NSZoneFree(NSDefaultMallocZone(), retval);
+    {
-  callframe_free(cframe);
+      NSZoneFree(NSDefaultMallocZone(), ctxt->datToFree);
+      ctxt->datToFree = 0;
+    }
 
   return;
 }