MYNetwork icon indicating copy to clipboard operation
MYNetwork copied to clipboard

Fix for TCPConnection/TCPStream Relationship ARC Crash

Open levigroker opened this issue 7 years ago • 0 comments

I spent some time tracking this down (since I wasn't happy with the solution other contributors had suggested (see #9 ), and I believe I understand the issue and implications.

Specifically:

  • TCPConnection adds newly open connections to an internal static array sAllConnections which is the only thing preventing ARC from deallocating the instances.
  • TCPConnection maintains a strong _reader and _writer (both TCPStream objects) which in turn also have strong reference to the connection.
  • When a TCPStream object receives an EOF it calls TCPConnection -_streamGotEOF: with self to inform its connection to disconnect. This ultimately causes the connection to call disconnect on both its _reader and _writer streams via _streamCanClose:.
  • TCPStream -disconnect calls TCPConnection -_streamDisconnected: with self to let its connection know it is closed.
  • Once both of the TCPConnection streams are closed, the connection calls _closed which removes itself from the static array sAllConnections, causing it to be deallocated.
  • At this point any subsequent calls to the TCPConnection instance are to a deallocated instance and a crash occurs.

This all manifests itself in the TCPConnection.m _streamGotEOF: method.

My fix is to strongly reference self so self is available in the current scope. This is similar to what MYDeferDealloc does in a non-ARC setting.

Proposed fix:

TCPConnection.m

- (void) _streamGotEOF: (TCPStream*)stream
{
	LogTo(TCP,@"%@ got EOF on %@",self,stream);
	[stream disconnect];
	if( _status == kTCP_Closing ) {
		// Strongly reference `self` in this scope, so when `_closed` (ultimately called by `_streamCanClose:`) removes `self` from `sAllConnections` we are not immediately deallocated (and subsequently crash when attempting to call `[self _checkIfClosed]`)
		__strong typeof(self) sSelf = self;
		[sSelf _streamCanClose: stream];
		[sSelf _checkIfClosed];
	} else {
		[self _stream: stream
			 gotError: [NSError errorWithDomain: NSPOSIXErrorDomain code: ECONNRESET userInfo: nil]];
	}
}

levigroker avatar Feb 27 '18 23:02 levigroker