diff --git a/ChangeLog b/ChangeLog index 911b55448..5bf0db290 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,10 @@ +2013-11-21 Richard Frith-Macdonald + + * Source/GSFileHandle.m: Do an -sslDisconnect while finalising + (before underlying network connection is closed). + * Source/NSFileHandle.m: Add assertion to check that TLS has been + shut down by concrete subclass dealloc. + 2013-11-20 Richard Frith-Macdonald * Source/NSUserDefaults.m: Fix for bug 40620 diff --git a/Source/GSFileHandle.m b/Source/GSFileHandle.m index 02c63a0d4..6e898d5ca 100644 --- a/Source/GSFileHandle.m +++ b/Source/GSFileHandle.m @@ -214,8 +214,7 @@ static NSString* NotificationKey = @"NSFileHandleNotificationKey"; [self ignoreWriteDescriptor]; #if USE_ZLIB - /* - * The gzDescriptor should always be closed when we have done with it. + /* The gzDescriptor should always be closed when we have done with it. */ if (gzDescriptor != 0) { @@ -223,6 +222,14 @@ static NSString* NotificationKey = @"NSFileHandleNotificationKey"; gzDescriptor = 0; } #endif + + /* Ensure any SSL/TLS connection has been properly shut down. + */ + [self sslDisconnect]; + + /* Close file descriptor if necessary (setting correct non-blocking + * characteristics since we may have changed them). + */ if (descriptor != -1) { [self setNonBlocking: wasNonBlocking]; @@ -232,6 +239,7 @@ static NSString* NotificationKey = @"NSFileHandleNotificationKey"; descriptor = -1; } } + [super finalize]; } // Initializing a GSFileHandle Object diff --git a/Source/NSFileHandle.m b/Source/NSFileHandle.m index b4c583440..2de11c55e 100644 --- a/Source/NSFileHandle.m +++ b/Source/NSFileHandle.m @@ -953,29 +953,22 @@ GSTLSHandlePush(gnutls_transport_ptr_t handle, const void *buffer, size_t len) - (void) closeFile { - [self sslDisconnect]; + [self sslDisconnect]; // Shut down TLS before closing socket [super closeFile]; } - (void) dealloc { - // TLS may need to read data during teardown, and we need to wait for it. - [self setNonBlocking: NO]; - // Don't DESTROY ivars below. First release them, then set nil, because - // `session' may need this back-reference during TLS teardown. - TEST_RELEASE(opts); - TEST_RELEASE(session); - opts = nil; - session = nil; + /* Any TLS connection needs to be shut down before the network connection + * is closed, which means that the concrete subclass must do that. + * Therefore, the session should be inactive by the time we get here. + */ + NSAssert(NO == [session active], NSInternalInconsistencyException); + DESTROY(session); + DESTROY(opts); [super dealloc]; } -- (void) finalize -{ - [self sslDisconnect]; - [super finalize]; -} - - (NSInteger) read: (void*)buf length: (NSUInteger)len { if (YES == [session active]) @@ -987,7 +980,14 @@ GSTLSHandlePush(gnutls_transport_ptr_t handle, const void *buffer, size_t len) - (void) sslDisconnect { - [session disconnect]; + if (nil != session) + { + // TLS may need to read data during teardown, and we need to wait for it. + [self setNonBlocking: NO]; + [session disconnect]; + DESTROY(session); + } + DESTROY(opts); } - (BOOL) sslHandshakeEstablished: (BOOL*)result outgoing: (BOOL)isOutgoing diff --git a/Source/win32/GSFileHandle.m b/Source/win32/GSFileHandle.m index a297c599d..ce830a065 100644 --- a/Source/win32/GSFileHandle.m +++ b/Source/win32/GSFileHandle.m @@ -270,6 +270,11 @@ getAddr(NSString* name, NSString* svc, NSString* pcl, struct sockaddr_in *sin) gzclose(gzDescriptor); } #endif + + /* Ensure any SSL/TLS connection has been properly shut down. + */ + [self sslDisconnect]; + if (descriptor != -1) { [self setNonBlocking: wasNonBlocking];