facebookincubator / SocketRocket

A conforming Objective-C WebSocket client library.
Other
9.54k stars 2.01k forks source link

SRProxyConnect Crash #587

Open LoveLifeLoveSelf opened 6 years ago

LoveLifeLoveSelf commented 6 years ago

EXC_BAD_ACCESS (SIGBUS)

0 libobjc.A.dylib objc_msgSend + 16 1 xxxxxx -[SRProxyConnect _failWithError:] (SRProxyConnect.m:140) 2 xxxxxx -[SRProxyConnect stream:handleEvent:] (SRProxyConnect.m:0) 3 CoreFoundation _signalEventSync + 212 4 CoreFoundation _cfstream_shared_signalEventSync + 460 5 CoreFoundation CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION + 24 6 CoreFoundation CFRunLoopDoSources0 + 276 7 CoreFoundation CFRunLoopRun + 1204 8 CoreFoundation CFRunLoopRunSpecific + 552 9 Foundation -[NSRunLoop(NSRunLoop) runMode:beforeDate:] + 304 10 xxxxxx -[SRRunLoopThread main] (SRRunLoopThread.m:70) 11 Foundation NSThreadstart__ + 1040 12 libsystem_pthread.dylib _pthread_body + 272 13 libsystem_pthread.dylib _pthread_body + 0

this crash found online

LoveLifeLoveSelf commented 6 years ago

@kommen @lukeredpath @xaviershay @danielrhammond @mk Ask for help,thanks! #

MikeWeller commented 5 years ago

The SRProxyConnect class is rather unsafe around NSStream lifetime and event dispatches. There have been similar issues in the past with SRWebSocket but those fixed haven't made it to SRProxyConnect.

The input and output streams must both be scheduled on the SR_networkRunLoop run loop (currently only the inputstream is, with a commented out line for the output stream which has been there since the class was created). In addition, the closing, nil-ing out of instance variables, and nil-ing out of the delegates for those streams must be performed on that same runloop, otherwise the streams themselves can end up getting deallocated while events are being processed, or the delegate (SRProxyConnect itself) can end up getting deallocated while events are being processed, since the streams keep a non-retained reference to it.

So, although I haven't got round to submitting my fix for this:

MikeWeller commented 5 years ago

Pasting some raw notes from a crash analysis I did:

SRProxyConnect crashes

Two main variants of the crash we see:

BUS_ADRALN at 0xffffffff89abcdef we're trying to jump to this bad address from failWithError:, which comes from the value of the _completion instance variable from an earlier call to openNetworkStreamWithCompletion. So the completion is bad, so the memory of the object is bad. The delegate 'self' in this case is 0x00000001d40eba80 which looks like it could be a valid pointer value, but maybe dangling/deallocated/reallocated/etc.

SEGV_ACCERR at 0xd000000010d449a8 this pointer is from the x0 register which is the 'self' value itself in a call to -[SRProxyConnect stream:handleEvent:], pointer itself looks bad which would suggest the calling stream itself has been deallocated and its delegate property/ivar gets trashed.

So SRProxyConnect is having its stream:handleEvent: called by the stream, since it sets itself as a delegate, but it looks like the self delegate pointer value itself is invalid which suggests it is the stream itself that has been deallocated, its memory trashed and it ends up dispatching a delegate call to a runloop with a bad pointer. Sometimes the pointer is semi-valid or dangling and we get further along but try calling a bad _completion on SRProxyConnect. Sometimes the pointer is entirely bad and we get an immediate SEGV_ACCERR when we try and load any instance data from it.

The event code switch is optimized/merged for the two NSStreamEventEndEncountered and NSStreamEventErrorOccurred cases so we don't have an exact source location in the symbolicated crash, but the event code variable itself comes from register x3 one frame up and isn't mutated, and has a value of 0x0000000000000008 (we've lost it in the BYS_ADRALN one apparently), which is NSStreamEventErrorOccurred = 1UL << 3

The streamError that is pulled from aStream ends up in register x2 in the crashing frame, if it's null we reassign by creating a new one (SRHTTPErrorWithCodeDescription(500, 2132,@"Proxy Error");), but it has a value of x2: 0x0000000000007803 which suggests it was bad when we got it from the stream itself.

So everything points to the stream itself being bad with bad data members, calling us (the delegate) via its bad/dangling delegate pointer, as well as its other members like the stream error being invalid.

MikeWeller commented 5 years ago

To further clarify what I think is happening:

codingingBoy commented 1 year ago

So is there a conclusion for this? I received more and more this crash for my app these days.