Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Update RunLoop/AsyncUdpSocket.m #49

Closed
wants to merge 1 commit into from

3 participants

@gghose

Found a pretty bad bug. The queues aren't safe because you can potentially remove objects at position 0 (dequeuing) at the same time you're adding objects. The solution is to put in a lock whenever you manipulate the queue. This got rid of consistent crashes for me.

So I've added NSLock objects theSendQueueLock and theReceiveQueueLock. (Note you'll need to declare them in the corresponding AsyncUdpSocket.h).

This is the only part of the library I'm actually using, so I don't know how pervasive this queuing problem is.

@evands

The RunLoop based Async socket classes are explicitly not threadsafe; you need to always access them on the same run loop to be safe.

@dvor
Collaborator

@evands is right, RunLoop socket library isn't thread safe. Closing this issue.

@dvor dvor closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 28, 2012
  1. @gghose

    Update RunLoop/AsyncUdpSocket.m

    gghose authored
This page is out of date. Refresh to see the latest.
Showing with 16 additions and 6 deletions.
  1. +16 −6 RunLoop/AsyncUdpSocket.m
View
22 RunLoop/AsyncUdpSocket.m
@@ -205,10 +205,12 @@ - (id)initWithDelegate:(id)delegate userData:(long)userData enableIPv4:(BOOL)ena
theSendQueue = [[NSMutableArray alloc] initWithCapacity:SENDQUEUE_CAPACITY];
theCurrentSend = nil;
theSendTimer = nil;
+ theSendQueueLock = [[NSLock alloc] init];
theReceiveQueue = [[NSMutableArray alloc] initWithCapacity:RECEIVEQUEUE_CAPACITY];
theCurrentReceive = nil;
theReceiveTimer = nil;
+ theReceiveQueueLock = [[NSLock alloc] init];
// Socket context
theContext.version = 0;
@@ -308,7 +310,8 @@ - (id)initIPv6
- (void) dealloc
{
[self close];
-
+ [theSendQueueLock release];
+ [theReceiveQueueLock release];
[NSObject cancelPreviousPerformRequestsWithTarget:theDelegate selector:@selector(onUdpSocketDidClose:) object:self];
[NSObject cancelPreviousPerformRequestsWithTarget:self];
}
@@ -1662,8 +1665,9 @@ - (BOOL)sendData:(NSData *)data withTimeout:(NSTimeInterval)timeout tag:(long)ta
if(![self isConnected]) return NO;
AsyncSendPacket *packet = [[AsyncSendPacket alloc] initWithData:data address:nil timeout:timeout tag:tag];
-
+ [theSendQueueLock lock];
[theSendQueue addObject:packet];
+ [theSendQueueLock unlock];
[self scheduleDequeueSend];
return YES;
@@ -1693,8 +1697,9 @@ - (BOOL)sendData:(NSData *)data
packet = [[AsyncSendPacket alloc] initWithData:data address:address6 timeout:timeout tag:tag];
else
return NO;
-
+ [theSendQueueLock lock];
[theSendQueue addObject:packet];
+ [theSendQueueLock unlock];
[self scheduleDequeueSend];
return YES;
@@ -1716,8 +1721,9 @@ - (BOOL)sendData:(NSData *)data toAddress:(NSData *)remoteAddr withTimeout:(NSTi
return NO;
AsyncSendPacket *packet = [[AsyncSendPacket alloc] initWithData:data address:remoteAddr timeout:timeout tag:tag];
-
+ [theSendQueueLock lock];
[theSendQueue addObject:packet];
+ [theSendQueueLock unlock];
[self scheduleDequeueSend];
return YES;
@@ -1790,7 +1796,9 @@ - (void)maybeDequeueSend
{
// Dequeue next send packet
theCurrentSend = [theSendQueue objectAtIndex:0];
+ [theSendQueueLock lock];
[theSendQueue removeObjectAtIndex:0];
+ [theSendQueueLock unlock];
// Start time-out timer.
if(theCurrentSend->timeout >= 0.0)
@@ -1941,8 +1949,9 @@ - (void)receiveWithTimeout:(NSTimeInterval)timeout tag:(long)tag
if(theFlags & kDidClose) return;
AsyncReceivePacket *packet = [[AsyncReceivePacket alloc] initWithTimeout:timeout tag:tag];
-
+ [theReceiveQueueLock lock];
[theReceiveQueue addObject:packet];
+ [theReceiveQueueLock unlock];
[self scheduleDequeueReceive];
}
@@ -2002,8 +2011,9 @@ - (void)maybeDequeueReceive
{
// Dequeue next receive packet
theCurrentReceive = [theReceiveQueue objectAtIndex:0];
+ [theReceiveQueueLock lock];
[theReceiveQueue removeObjectAtIndex:0];
-
+ [theReceiveQueueLock unlock];
// Start time-out timer.
if (theCurrentReceive->timeout >= 0.0)
{
Something went wrong with that request. Please try again.