Changes to avoid possible hang in connection reply mode due to race condition.

This commit is contained in:
Richard Frith-Macdonald 2017-12-21 13:51:01 +00:00
parent 38fc2bde82
commit 4ce7a25541
5 changed files with 209 additions and 224 deletions

View file

@ -1,3 +1,37 @@
2017-12-21 Richard Frith-Macdonald <rfm@gnu.org>
* Source/NSSocketPort.m:
* Source/NSMessagePort.m:
* Source/win32/NSMessagePort.m:
There is a possible race condition when attempting to write a message
to a port; the port handle is added to the run loop and then the loop
is entered, but if an event occurs in the loop which invalidates the
handle and causes it to be removed from the loop before the lower
level code to wait for I/O is entered, the loop may never receive an
event on the handle and could wait indefinitely.
The solution is not to remove the handle from the loop when it is
invalidated or when an event occurs on an invalidated handle, but
only remove it in the method which added it. Of course, this means
we must make certain to remove it when we should, which means that
the loop must be run inside an exception handler which makes sure to
remove the handle from the loop before re-raising the exception.
2017-12-20 Richard Frith-Macdonald <rfm@gnu.org>
* configure.ac: Remove obsolete --enable-objc-nonfragile-abi flag
when building for ng.
* configure: regenerate
2017-12-19 Richard Frith-Macdonald <rfm@gnu.org>
* Version: 1.25.1 release
* Documentation/ReleaseNotes.gsdoc: update for release
* Documentation/news.texi: update for release
* ANNOUNCE: regenerate
* INSTALL: regenerate
* NEWS: regenerate
* README: regenerate
2017-12-18 Graham Lee <graham@iamleeg.com>
* Source/NSJSONSerialization.m: Fix for bug #12 on github. This

View file

@ -1914,8 +1914,9 @@ failure:
if (att != nil)
{
NSMutableDictionary *mAtt = [att mutableCopy];
IF_NO_GC(AUTORELEASE(mAtt));
NSMutableDictionary *mAtt = [att mutableCopy];
IF_NO_GC(AUTORELEASE(mAtt));
/*
* We have created a new file - so we attempt to make it's
* attributes match that of the original.

View file

@ -345,6 +345,30 @@ static Class runLoopClass;
}
}
- (void) _add: (NSRunLoop*)l
{
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSDefaultRunLoopMode];
}
- (void) _rem: (NSRunLoop*)l
{
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSDefaultRunLoopMode
all: NO];
}
- (BOOL) connectToPort: (NSMessagePort*)aPort beforeDate: (NSDate*)when
{
NSRunLoop *l;
@ -383,7 +407,6 @@ static Class runLoopClass;
return NO; /* impossible. */
}
name = [aPort _name];
memset(&sockAddr, '\0', sizeof(sockAddr));
sockAddr.sun_family = AF_LOCAL;
@ -402,14 +425,7 @@ static Class runLoopClass;
state = GS_H_TRYCON;
l = [NSRunLoop currentRunLoop];
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSDefaultRunLoopMode];
[self _add: l];
while (valid == YES && state == GS_H_TRYCON
&& [when timeIntervalSinceNow] > 0)
@ -417,14 +433,7 @@ static Class runLoopClass;
[l runMode: NSConnectionReplyMode beforeDate: when];
}
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSDefaultRunLoopMode
all: NO];
[self _rem: l];
if (state == GS_H_TRYCON)
{
@ -1023,14 +1032,7 @@ static Class runLoopClass;
IF_NO_GC(RETAIN(self);)
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSDefaultRunLoopMode];
[self _add: l];
while (valid == YES
&& [wMsgs indexOfObjectIdenticalTo: components] != NSNotFound
@ -1041,19 +1043,16 @@ static Class runLoopClass;
M_LOCK(myLock);
}
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSDefaultRunLoopMode
all: NO];
[self _rem: l];
if ([wMsgs indexOfObjectIdenticalTo: components] == NSNotFound)
{
sent = YES;
}
else
{
[wMsgs removeObjectIdenticalTo: components];
}
M_UNLOCK(myLock);
NSDebugMLLog(@"NSMessagePort_details",
@"Message send 0x%"PRIxPTR" on 0x%"PRIxPTR" status %d",
@ -1698,10 +1697,6 @@ typedef struct {
else if (type == ET_RPORT) t = "rport";
else t = "unknown";
NSLog(@"No handle for event %s on descriptor %d", t, desc);
[[runLoopClass currentRunLoop] removeEvent: extra
type: type
forMode: mode
all: YES];
}
else
{

View file

@ -425,7 +425,6 @@ static Class runLoopClass;
handle->desc = d;
handle->wMsgs = [NSMutableArray new];
handle->myLock = [GSLazyRecursiveLock new];
handle->valid = YES;
#if defined(_WIN32)
ev = (WSAEVENT)CreateEvent(NULL,NO,NO,NULL);
if (ev == WSA_INVALID_EVENT)
@ -440,6 +439,7 @@ static Class runLoopClass;
handle->inReplyMode = NO;
handle->readyToSend = YES;
#endif
handle->valid = YES;
return AUTORELEASE(handle);
}
@ -461,6 +461,70 @@ static Class runLoopClass;
}
}
- (void) _add: (NSRunLoop*)l
{
#if defined(_WIN32)
[l addEvent: (void*)(uintptr_t)event
type: ET_HANDLE
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)event
type: ET_HANDLE
watcher: self
forMode: NSDefaultRunLoopMode];
inReplyMode = YES;
#else
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_EDESC
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSDefaultRunLoopMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_EDESC
watcher: self
forMode: NSDefaultRunLoopMode];
#endif
}
- (void) _rem: (NSRunLoop*)l
{
#if defined(_WIN32)
[l removeEvent: (void*)(uintptr_t)event
type: ET_HANDLE
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)event
type: ET_HANDLE
forMode: NSDefaultRunLoopMode
all: NO];
inReplyMode = NO;
#else
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_EDESC
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSDefaultRunLoopMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_EDESC
forMode: NSDefaultRunLoopMode
all: NO];
#endif
}
- (BOOL) connectToPort: (NSSocketPort*)aPort beforeDate: (NSDate*)when
{
NSArray *addrs;
@ -564,72 +628,24 @@ static Class runLoopClass;
state = GS_H_TRYCON;
l = [NSRunLoop currentRunLoop];
#if defined(_WIN32)
NSAssert(event != WSA_INVALID_EVENT, @"Socket without win32 event!");
[l addEvent: (void*)(uintptr_t)event
type: ET_HANDLE
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)event
type: ET_HANDLE
watcher: self
forMode: NSDefaultRunLoopMode];
inReplyMode = YES;
#else
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_EDESC
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSDefaultRunLoopMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_EDESC
watcher: self
forMode: NSDefaultRunLoopMode];
#endif
[self _add: l];
while (valid == YES && state == GS_H_TRYCON
&& [when timeIntervalSinceNow] > 0)
{
M_UNLOCK(myLock);
[l runMode: NSConnectionReplyMode beforeDate: when];
NS_DURING
[l runMode: NSConnectionReplyMode beforeDate: when];
NS_HANDLER
M_LOCK(myLock);
[self _rem: l];
M_UNLOCK(myLock);
[localException raise];
NS_ENDHANDLER
M_LOCK(myLock);
}
#if defined(_WIN32)
[l removeEvent: (void*)(uintptr_t)event
type: ET_HANDLE
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)event
type: ET_HANDLE
forMode: NSDefaultRunLoopMode
all: NO];
inReplyMode = NO;
#else
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_EDESC
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSDefaultRunLoopMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_EDESC
forMode: NSDefaultRunLoopMode
all: NO];
#endif
[self _rem: l];
if (state == GS_H_TRYCON)
{
@ -703,8 +719,18 @@ static Class runLoopClass;
- (void) finalize
{
[self invalidate];
(void)close(desc);
desc = -1;
#if defined(_WIN32)
if (event != WSA_INVALID_EVENT)
{
WSACloseEvent(event);
event = WSA_INVALID_EVENT;
}
#endif
if (desc >= 0)
{
(void)close(desc);
desc = -1;
}
}
- (void) invalidate
@ -714,37 +740,11 @@ static Class runLoopClass;
M_LOCK(myLock);
if (valid == YES)
{
NSRunLoop *l;
valid = NO;
l = [runLoopClass currentRunLoop];
#if defined(_WIN32)
[l removeEvent: (void*)(uintptr_t)event
type: ET_HANDLE
forMode: nil
all: YES];
#else
[l removeEvent: (void*)(uintptr_t)desc
type: ET_RDESC
forMode: nil
all: YES];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: nil
all: YES];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_EDESC
forMode: nil
all: YES];
#endif
NSDebugMLLog(@"GSTcpHandle",
@"invalidated 0x%"PRIxPTR, (NSUInteger)self);
[[self recvPort] removeHandle: self];
[[self sendPort] removeHandle: self];
#if defined(_WIN32)
WSACloseEvent(event);
event = WSA_INVALID_EVENT;
#endif
}
M_UNLOCK(myLock);
}
@ -821,7 +821,7 @@ static Class runLoopClass;
[self invalidate];
return;
}
res = 0; /* Interrupted - continue */
res = 0; /* Interrupted - continue */
}
NSDebugMLLog(@"GSTcpHandle",
@"read %d bytes on 0x%"PRIxPTR, res, (NSUInteger)self);
@ -1228,17 +1228,15 @@ static Class runLoopClass;
#if defined(_WIN32)
WSANETWORKEVENTS ocurredEvents;
/* If we have been invalidated then we should ignore this
* event and remove ourself from the runloop.
/* If we have been invalidated then we should ignore this event.
*/
if (NO == valid || desc == INVALID_SOCKET)
if (NO == valid)
{
NSRunLoop *l = [runLoopClass currentRunLoop];
[l removeEvent: data
type: ET_HANDLE
forMode: mode
all: YES];
return;
}
if (INVALID_SOCKET == desc)
{
[self invalidate];
return;
}
@ -1315,21 +1313,15 @@ static Class runLoopClass;
#else
/* If we have been invalidated then we should ignore this
* event and remove ourself from the runloop.
/* If we have been invalidated then we should ignore this event.
*/
if (NO == valid || desc < 0)
if (NO == valid)
{
NSRunLoop *l = [runLoopClass currentRunLoop];
[l removeEvent: data
type: ET_WDESC
forMode: mode
all: YES];
[l removeEvent: data
type: ET_EDESC
forMode: mode
all: YES];
return;
}
if (desc < 0)
{
[self invalidate];
return;
}
@ -1364,89 +1356,46 @@ static Class runLoopClass;
IF_NO_GC(RETAIN(self);)
#if defined(_WIN32)
NSAssert(event != WSA_INVALID_EVENT, @"Socket without win32 event!");
[l addEvent: (void*)(uintptr_t)event
type: ET_HANDLE
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)event
type: ET_HANDLE
watcher: self
forMode: NSDefaultRunLoopMode];
inReplyMode = YES;
#else
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_EDESC
watcher: self
forMode: NSConnectionReplyMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_WDESC
watcher: self
forMode: NSDefaultRunLoopMode];
[l addEvent: (void*)(uintptr_t)desc
type: ET_EDESC
watcher: self
forMode: NSDefaultRunLoopMode];
#endif
[self _add: l];
while (valid == YES
&& [wMsgs indexOfObjectIdenticalTo: components] != NSNotFound
&& [when timeIntervalSinceNow] > 0)
{
M_UNLOCK(myLock);
NS_DURING
#if defined(_WIN32)
if (readyToSend)
{
[self receivedEventWrite];
}
else
{
[l runMode: NSConnectionReplyMode beforeDate: when];
}
if (readyToSend)
{
[self receivedEventWrite];
}
else
{
[l runMode: NSConnectionReplyMode beforeDate: when];
}
#else
[l runMode: NSConnectionReplyMode beforeDate: when];
[l runMode: NSConnectionReplyMode beforeDate: when];
#endif
NS_HANDLER
M_LOCK(myLock);
[self _rem: l];
M_UNLOCK(myLock);
[localException raise];
NS_ENDHANDLER
M_LOCK(myLock);
}
#if defined(_WIN32)
[l removeEvent: (void*)(uintptr_t)event
type: ET_HANDLE
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)event
type: ET_HANDLE
forMode: NSDefaultRunLoopMode
all: NO];
inReplyMode = NO;
#else
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_EDESC
forMode: NSConnectionReplyMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_WDESC
forMode: NSDefaultRunLoopMode
all: NO];
[l removeEvent: (void*)(uintptr_t)desc
type: ET_EDESC
forMode: NSDefaultRunLoopMode
all: NO];
#endif
[self _rem: l];
if ([wMsgs indexOfObjectIdenticalTo: components] == NSNotFound)
{
sent = YES;
}
else
{
[wMsgs removeObjectIdenticalTo: components];
}
M_UNLOCK(myLock);
NSDebugMLLog(@"GSTcpHandle",
@"Message send 0x%"PRIxPTR" on 0x%"PRIxPTR" status %d",
@ -2209,10 +2158,6 @@ static Class tcpPortClass;
else if (type == ET_RPORT) t = "rport";
else t = "unknown";
NSLog(@"No handle for event %s on descriptor %d", t, desc);
[[runLoopClass currentRunLoop] removeEvent: extra
type: type
forMode: mode
all: YES];
}
else
{

View file

@ -909,10 +909,6 @@ again:
{
NSDebugMLLog(@"NSMessagePort",
@"got event on invalidated port 0x%x in mode %@", self, mode);
[[NSRunLoop currentRunLoop] removeEvent: data
type: type
forMode: mode
all: YES];
}
RELEASE(self);
}
@ -1109,7 +1105,21 @@ again:
&& [when timeIntervalSinceNow] > 0)
{
M_UNLOCK(this->lock);
[loop runMode: NSConnectionReplyMode beforeDate: when];
NS_DURING
[loop runMode: NSConnectionReplyMode beforeDate: when];
NS_HANDLER
M_LOCK(this->lock);
[loop removeEvent: (void*)(uintptr_t)this->wEvent
type: ET_HANDLE
forMode: NSConnectionReplyMode
all: NO];
[loop removeEvent: (void*)(uintptr_t)this->wEvent
type: ET_HANDLE
forMode: NSDefaultRunLoopMode
all: NO];
M_UNLOCK(this->lock);
[localException raise];
NS_ENDHANDLER
M_LOCK(this->lock);
}