Fix NSURLSession memory management of libdispatch objects and overrel…

…ease in GSHTTPURLProtocol.
gnustep · Jan 13, 2023 · c2b2785 · c2b2785
1 parent eea71ab
commit c2b2785
Show file tree

Hide file tree

Showing 7 changed files with 48 additions and 13 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,14 @@
+2023-01-13 Frederik Seiffert <[email protected]>
+
+	* Source/GSEasyHandle.m:
+	* Source/GSMultiHandle.m:
+	* Source/GSTimeoutSource.h:
+	* Source/GSTimeoutSource.m:
+	* Source/NSURLSession.m:
+	Fix NSURLSession memory management of libdispatch objects.
+	* Source/GSHTTPURLProtocol.m:
+	Fix overrelease.
+
 2023-01-13 Frederik Seiffert <[email protected]>
 
 	* Headers/Foundation/NSURLSession.h:

diff --git a/Source/GSEasyHandle.m b/Source/GSEasyHandle.m
@@ -195,6 +195,7 @@ - (void) dealloc
   curl_slist_free_all(_headerList);
   free(_errorBuffer);
   DESTROY(_config);
+  [_timeoutTimer cancel];
   DESTROY(_timeoutTimer);
   DESTROY(_URL);
   [super dealloc];
@@ -217,6 +218,7 @@ - (GSTimeoutSource*) timeoutTimer
 
 - (void) setTimeoutTimer: (GSTimeoutSource*)timer
 {
+  [_timeoutTimer cancel];
   ASSIGN(_timeoutTimer, timer);
 }
 
@@ -234,10 +236,13 @@ - (void) resetTimer
 {
   // simply create a new timer with the same queue, timeout and handler
   // this must cancel the old handler and reset the timer
-  DESTROY(_timeoutTimer);
-  _timeoutTimer = [[GSTimeoutSource alloc] initWithQueue: [_timeoutTimer queue]
-                                            milliseconds: [_timeoutTimer milliseconds]
-                                                 handler: [_timeoutTimer handler]];
+  if (_timeoutTimer) {
+    [_timeoutTimer cancel];
+    _timeoutTimer = [[GSTimeoutSource alloc] initWithQueue: [_timeoutTimer queue]
+                                              milliseconds: [_timeoutTimer milliseconds]
+                                                   handler: [_timeoutTimer handler]];
+    DESTROY(_timeoutTimer);
+  }
 }
 
 - (void) setupCallbacks 

diff --git a/Source/GSHTTPURLProtocol.m b/Source/GSHTTPURLProtocol.m
@@ -592,17 +592,14 @@ - (void) configureEasyHandleForRequest: (NSURLRequest*)request
   [hh addEntriesFromDictionary: 
     [self transformLowercaseKeyForHTTPHeaders: HTTPHeaders]];
 
-  NSArray *curlHeaders = [self curlHeadersForHTTPHeaders: hh];
+  NSMutableArray *curlHeaders = [self curlHeadersForHTTPHeaders: hh];
   if ([[request HTTPMethod] isEqualToString:@"POST"] 
     && [[request HTTPBody] length] > 0
     && [request valueForHTTPHeaderField: @"Content-Type"] == nil) 
     {
-      NSMutableArray *temp = [curlHeaders mutableCopy];
-      [temp addObject: @"Content-Type:application/x-www-form-urlencoded"];
-      curlHeaders = temp;
+      [curlHeaders addObject: @"Content-Type:application/x-www-form-urlencoded"];
     }
   [_easyHandle setCustomHeaders: curlHeaders];
-  RELEASE(curlHeaders);
 
   NSInteger        timeoutInterval = [request timeoutInterval] * 1000;
   GSTimeoutSource  *timeoutTimer;
@@ -874,7 +871,7 @@ - (NSDictionary*) transformLowercaseKeyForHTTPHeaders: (NSDictionary*)HTTPHeader
 // expects.
 //
 // - SeeAlso: https://curl.haxx.se/libcurl/c/CURLOPT_HTTPHEADER.html
-- (NSArray*) curlHeadersForHTTPHeaders: (NSDictionary*)HTTPHeaders 
+- (NSMutableArray*) curlHeadersForHTTPHeaders: (NSDictionary*)HTTPHeaders 
 {
   NSMutableArray *result = [NSMutableArray array];
   NSMutableSet   *names = [NSMutableSet set];
@@ -951,7 +948,7 @@ - (NSArray*) curlHeadersForHTTPHeaders: (NSDictionary*)HTTPHeaders
       [result addObject: [NSString stringWithFormat: @"%@:", k]];
     }
 
-  return AUTORELEASE([result copy]);
+  return result;
 }
 
 // Any header values that should be passed to libcurl

diff --git a/Source/GSMultiHandle.m b/Source/GSMultiHandle.m
@@ -108,8 +108,11 @@ - (void) dealloc
   NSEnumerator   *e;
   GSEasyHandle   *handle;
 
+  [_timeoutSource cancel];
   DESTROY(_timeoutSource);
 
+  dispatch_release(_queue);
+
   e = [_easyHandles objectEnumerator];
   while (nil != (handle = [e nextObject]))
     {
@@ -193,10 +196,12 @@ - (void) updateTimeoutTimerToValue: (NSInteger)value
   // of milliseconds.
   if (-1 == value)
     {
+      [_timeoutSource cancel];
       DESTROY(_timeoutSource);
     }   
   else if (0 == value) 
     {
+      [_timeoutSource cancel];
       DESTROY(_timeoutSource);
       dispatch_async(_queue, 
         ^{
@@ -207,6 +212,7 @@ - (void) updateTimeoutTimerToValue: (NSInteger)value
     {
       if (nil == _timeoutSource || value != [_timeoutSource milliseconds]) 
         {
+          [_timeoutSource cancel];
           DESTROY(_timeoutSource);
           _timeoutSource = [[GSTimeoutSource alloc] initWithQueue: _queue
                                                      milliseconds: value
@@ -438,12 +444,14 @@ - (void) dealloc
   if (_readSource) 
     {
       dispatch_source_cancel(_readSource);
+      dispatch_release(_readSource);
     }
   _readSource = NULL;
 
   if (_writeSource) 
     {
       dispatch_source_cancel(_writeSource);
+      dispatch_release(_writeSource);
     }
   _writeSource = NULL;
   [super dealloc];

diff --git a/Source/GSTimeoutSource.h b/Source/GSTimeoutSource.h
@@ -17,6 +17,8 @@
   dispatch_block_t   _handler;
 }
 
+- (void) cancel;
+
 - (NSInteger) milliseconds;
 
 - (dispatch_queue_t) queue;

diff --git a/Source/GSTimeoutSource.m b/Source/GSTimeoutSource.m
@@ -9,7 +9,7 @@ - (instancetype) initWithQueue: (dispatch_queue_t)queue
   if (nil != (self = [super init])) 
     {
       _queue = queue;
-      _handler = handler;
+      _handler = Block_copy(handler);
       _milliseconds = milliseconds;
       _rawSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, _queue);
 
@@ -26,10 +26,20 @@ - (instancetype) initWithQueue: (dispatch_queue_t)queue
 
 - (void) dealloc 
 {
-  dispatch_source_cancel(_rawSource);
+  [self cancel];
+  Block_release(_handler);
   [super dealloc];
 }
 
+- (void) cancel
+{
+  if (_rawSource) {
+    dispatch_source_cancel(_rawSource);
+    dispatch_release(_rawSource);
+    _rawSource = NULL;
+  }
+}
+
 - (NSInteger) milliseconds
 {
   return _milliseconds;

diff --git a/Source/NSURLSession.m b/Source/NSURLSession.m
@@ -976,6 +976,7 @@ - (void) dealloc
   DESTROY(_protocolBag);
   DESTROY(_dataCompletionHandler);
   DESTROY(_knownBody);
+  dispatch_release(_workQueue);
   [super dealloc];
 }
 
@@ -1218,6 +1219,7 @@ - (id) copyWithZone: (NSZone*)zone
       copy->_state = _state;
       copy->_error = [_error copyWithZone: zone];
       copy->_session = _session;
+      dispatch_retain(_workQueue);
       copy->_workQueue = _workQueue;
       copy->_suspendCount  = _suspendCount;
       copy->_protocolLock = [_protocolLock copy];