Skip to content

Commit

Permalink
Fixing a bug where we would dispatch asynchronously onto another thre…
Browse files Browse the repository at this point in the history
…ad before reading errno if an error occurred when trying to connect. Because errno is bound to a thread, the error number will often be wrong, as GCD gives no guarantee that a dispatch will execute a block on the calling thread. We now capture the errno before dispatching.
  • Loading branch information
jakebromberg committed Jul 26, 2018
1 parent ec6c373 commit ca18553
Showing 1 changed file with 22 additions and 21 deletions.
43 changes: 22 additions & 21 deletions Source/GCD/GCDAsyncSocket.m
Original file line number Diff line number Diff line change
Expand Up @@ -1453,7 +1453,7 @@ - (BOOL)acceptOnInterface:(NSString *)inInterface port:(uint16_t)port error:(NSE
if (socketFD == SOCKET_NULL)
{
NSString *reason = @"Error in socket() function";
err = [self errnoErrorWithReason:reason];
err = [self errorWithErrno:errno reason:reason];

return SOCKET_NULL;
}
Expand All @@ -1466,7 +1466,7 @@ - (BOOL)acceptOnInterface:(NSString *)inInterface port:(uint16_t)port error:(NSE
if (status == -1)
{
NSString *reason = @"Error enabling non-blocking IO on socket (fcntl)";
err = [self errnoErrorWithReason:reason];
err = [self errorWithErrno:errno reason:reason];

LogVerbose(@"close(socketFD)");
close(socketFD);
Expand All @@ -1478,7 +1478,7 @@ - (BOOL)acceptOnInterface:(NSString *)inInterface port:(uint16_t)port error:(NSE
if (status == -1)
{
NSString *reason = @"Error enabling address reuse (setsockopt)";
err = [self errnoErrorWithReason:reason];
err = [self errorWithErrno:errno reason:reason];

LogVerbose(@"close(socketFD)");
close(socketFD);
Expand All @@ -1491,7 +1491,7 @@ - (BOOL)acceptOnInterface:(NSString *)inInterface port:(uint16_t)port error:(NSE
if (status == -1)
{
NSString *reason = @"Error in bind() function";
err = [self errnoErrorWithReason:reason];
err = [self errorWithErrno:errno reason:reason];

LogVerbose(@"close(socketFD)");
close(socketFD);
Expand All @@ -1504,7 +1504,7 @@ - (BOOL)acceptOnInterface:(NSString *)inInterface port:(uint16_t)port error:(NSE
if (status == -1)
{
NSString *reason = @"Error in listen() function";
err = [self errnoErrorWithReason:reason];
err = [self errorWithErrno:errno reason:reason];

LogVerbose(@"close(socketFD)");
close(socketFD);
Expand Down Expand Up @@ -1766,7 +1766,7 @@ - (BOOL)acceptOnUrl:(NSURL *)url error:(NSError **)errPtr;
if (socketFD == SOCKET_NULL)
{
NSString *reason = @"Error in socket() function";
err = [self errnoErrorWithReason:reason];
err = [self errorWithErrno:errno reason:reason];

return SOCKET_NULL;
}
Expand All @@ -1779,7 +1779,7 @@ - (BOOL)acceptOnUrl:(NSURL *)url error:(NSError **)errPtr;
if (status == -1)
{
NSString *reason = @"Error enabling non-blocking IO on socket (fcntl)";
err = [self errnoErrorWithReason:reason];
err = [self errorWithErrno:errno reason:reason];

LogVerbose(@"close(socketFD)");
close(socketFD);
Expand All @@ -1791,7 +1791,7 @@ - (BOOL)acceptOnUrl:(NSURL *)url error:(NSError **)errPtr;
if (status == -1)
{
NSString *reason = @"Error enabling address reuse (setsockopt)";
err = [self errnoErrorWithReason:reason];
err = [self errorWithErrno:errno reason:reason];

LogVerbose(@"close(socketFD)");
close(socketFD);
Expand All @@ -1804,7 +1804,7 @@ - (BOOL)acceptOnUrl:(NSURL *)url error:(NSError **)errPtr;
if (status == -1)
{
NSString *reason = @"Error in bind() function";
err = [self errnoErrorWithReason:reason];
err = [self errorWithErrno:errno reason:reason];

LogVerbose(@"close(socketFD)");
close(socketFD);
Expand All @@ -1817,7 +1817,7 @@ - (BOOL)acceptOnUrl:(NSURL *)url error:(NSError **)errPtr;
if (status == -1)
{
NSString *reason = @"Error in listen() function";
err = [self errnoErrorWithReason:reason];
err = [self errorWithErrno:errno reason:reason];

LogVerbose(@"close(socketFD)");
close(socketFD);
Expand Down Expand Up @@ -2638,7 +2638,7 @@ - (BOOL)bindSocket:(int)socketFD toInterface:(NSData *)connectInterface error:(N
if (result != 0)
{
if (errPtr)
*errPtr = [self errnoErrorWithReason:@"Error in bind() function"];
*errPtr = [self errorWithErrno:errno reason:@"Error in bind() function"];

return NO;
}
Expand All @@ -2654,7 +2654,7 @@ - (int)createSocket:(int)family connectInterface:(NSData *)connectInterface errP
if (socketFD == SOCKET_NULL)
{
if (errPtr)
*errPtr = [self errnoErrorWithReason:@"Error in socket() function"];
*errPtr = [self errorWithErrno:errno reason:@"Error in socket() function"];

return socketFD;
}
Expand Down Expand Up @@ -2693,6 +2693,7 @@ - (void)connectSocket:(int)socketFD address:(NSData *)address stateIndex:(int)aS
#pragma clang diagnostic warning "-Wimplicit-retain-self"

int result = connect(socketFD, (const struct sockaddr *)[address bytes], (socklen_t)[address length]);
int err = errno;

__strong GCDAsyncSocket *strongSelf = weakSelf;
if (strongSelf == nil) return_from_block;
Expand All @@ -2718,7 +2719,7 @@ - (void)connectSocket:(int)socketFD address:(NSData *)address stateIndex:(int)aS
// If there are no more sockets trying to connect, we inform the error to the delegate
if (strongSelf.socket4FD == SOCKET_NULL && strongSelf.socket6FD == SOCKET_NULL)
{
NSError *error = [strongSelf errnoErrorWithReason:@"Error in connect() function"];
NSError *error = [strongSelf errorWithErrno:err reason:@"Error in connect() function"];
[strongSelf didNotConnect:aStateIndex error:error];
}
}
Expand Down Expand Up @@ -2847,7 +2848,7 @@ - (BOOL)connectWithAddressUN:(NSData *)address error:(NSError **)errPtr
if (socketFD == SOCKET_NULL)
{
if (errPtr)
*errPtr = [self errnoErrorWithReason:@"Error in socket() function"];
*errPtr = [self errorWithErrno:errno reason:@"Error in socket() function"];

return NO;
}
Expand Down Expand Up @@ -2895,7 +2896,7 @@ - (BOOL)connectWithAddressUN:(NSData *)address error:(NSError **)errPtr
{
// TODO: Bad file descriptor
perror("connect");
NSError *error = [self errnoErrorWithReason:@"Error in connect() function"];
NSError *error = [self errorWithErrno:errno reason:@"Error in connect() function"];

dispatch_async(self->socketQueue, ^{ @autoreleasepool {

Expand Down Expand Up @@ -3446,13 +3447,13 @@ + (NSError *)gaiError:(int)gai_error
return [NSError errorWithDomain:@"kCFStreamErrorDomainNetDB" code:gai_error userInfo:userInfo];
}

- (NSError *)errnoErrorWithReason:(NSString *)reason
- (NSError *)errorWithErrno:(int)err reason:(NSString *)reason
{
NSString *errMsg = [NSString stringWithUTF8String:strerror(errno)];
NSString *errMsg = [NSString stringWithUTF8String:strerror(err)];
NSDictionary *userInfo = [NSDictionary dictionaryWithObjectsAndKeys:errMsg, NSLocalizedDescriptionKey,
reason, NSLocalizedFailureReasonErrorKey, nil];

return [NSError errorWithDomain:NSPOSIXErrorDomain code:errno userInfo:userInfo];
return [NSError errorWithDomain:NSPOSIXErrorDomain code:err userInfo:userInfo];
}

- (NSError *)errnoError
Expand Down Expand Up @@ -5269,7 +5270,7 @@ - (void)doReadData
if (errno == EWOULDBLOCK)
waiting = YES;
else
error = [self errnoErrorWithReason:@"Error in read() function"];
error = [self errorWithErrno:errno reason:@"Error in read() function"];

socketFDBytesAvailable = 0;
}
Expand Down Expand Up @@ -6241,7 +6242,7 @@ - (void)doWriteData
}
else
{
error = [self errnoErrorWithReason:@"Error in write() function"];
error = [self errorWithErrno:errno reason:@"Error in write() function"];
}
}
else
Expand Down Expand Up @@ -6334,7 +6335,7 @@ - (void)doWriteData

if (error)
{
[self closeWithError:[self errnoErrorWithReason:@"Error in write() function"]];
[self closeWithError:[self errorWithErrno:errno reason:@"Error in write() function"]];
}

// Do not add any code here without first adding a return statement in the error case above.
Expand Down

0 comments on commit ca18553

Please sign in to comment.