Websocket2 - #2
Conversation
| NSMutableArray *queue; | ||
| NSMutableArray *pingHandlers; | ||
| NSData *pingPayload; | ||
| GSURLSessionWebSocketMessageSendState active; |
There was a problem hiding this comment.
combine the send states
| GSURLSessionWebSocketLifecycleStateFailed = 3, | ||
| }; | ||
|
|
||
| typedef NS_ENUM(NSUInteger, GSURLSessionWebSocketReceivePhase) { |
There was a problem hiding this comment.
rename: WSReceiveContentType
| GSURLSessionWebSocketSendQueueEntryKindClose = 2, | ||
| }; | ||
|
|
||
| typedef NS_ENUM(NSUInteger, GSURLSessionWebSocketLifecyclePhase) { |
There was a problem hiding this comment.
rename: WSTaskConnectionState
| @class NSMutableArray; | ||
| @class NSMutableData; | ||
|
|
||
| typedef NS_ENUM(NSUInteger, GSURLSessionWebSocketSendQueueEntryKind) { |
There was a problem hiding this comment.
rename: WSMessageSendType
| GSURLSessionWebSocketMessageSendState active; | ||
| unsigned long long nextPingIdentifier; | ||
| BOOL frameStartRetryPending; | ||
| } GSURLSessionWebSocketSendState; |
There was a problem hiding this comment.
rename: WSTaskSendState
| NSData *closeReason; | ||
| BOOL closeFrameSent; | ||
| BOOL closeFrameReceived; | ||
| } GSURLSessionWebSocketLifecycleState; |
There was a problem hiding this comment.
rename: WSConnectionClosedContext
|
|
||
| typedef struct | ||
| { | ||
| GSURLSessionWebSocketLifecyclePhase phase; |
There was a problem hiding this comment.
move this to the parent ( NSURLSessionWebSocketTask )
| void (^completionHandler)(NSError *error); | ||
| GSURLSessionWebSocketSendQueueEntryKind kind; | ||
| NSURLSessionWebSocketMessageType dataType; | ||
| } GSURLSessionWebSocketSendQueueEntry; |
There was a problem hiding this comment.
rename: WSSendQueueEntry
| } GSURLSessionWebSocketSendQueueEntry; | ||
|
|
||
| static GSURLSessionWebSocketSendQueueEntry * | ||
| GSURLSessionWebSocketDataSendQueueEntryCreate( |
There was a problem hiding this comment.
If this throws while we're holding a mutex we're fucked. check.
| } | ||
| else | ||
| { | ||
| [NSException raise: NSInvalidArgumentException |
There was a problem hiding this comment.
make this branch an assertion, and ensure the caller prevents an invalid type. If the user has no way to supply an invalid type, we dont need to throw at all - the assertion is enough
| dispatch_async( | ||
| [[self _session] _workQueue], | ||
| ^{ | ||
| curl_easy_pause([self _easyHandle], CURLPAUSE_SEND_CONT); |
There was a problem hiding this comment.
Lets use a helper to resume send/recv so we don't have to touch curl internals everywhere.
| } | ||
| else | ||
| { | ||
| [internal->send.queue addObject: [NSValue valueWithPointer: entry]]; |
There was a problem hiding this comment.
happy branch first
| } | ||
|
|
||
| static void | ||
| GSURLSessionWebSocketCompleteSend( |
There was a problem hiding this comment.
rename: WSTaskNotifyCompletionHandler
| } | ||
|
|
||
| static void | ||
| GSURLSessionWebSocketCompleteOutstandingWork( |
There was a problem hiding this comment.
rename: WSTaskNotifyOutstandingCompletionHandlers
| { | ||
| GSURLSessionWebSocketDestroySendEntries(sendEntries, task, error); | ||
| GSURLSessionWebSocketCompleteReceiveHandlers(receiveHandlers, task, error); | ||
| GSURLSessionWebSocketCompletePingHandlers(task, pingHandlers, error); |
There was a problem hiding this comment.
all of these should be rename to match the WSTask naming and to indicate that this simply notifies the handlers instead of doing any complex completion logic
| GS_MUTEX_UNLOCK(GSIVar(task, mutex)); | ||
|
|
||
| GSURLSessionWebSocketCompleteReceive(task, handler, nil, error); | ||
| return 0; |
There was a problem hiding this comment.
why does this return anything?
| } | ||
|
|
||
| static void | ||
| GSURLSessionWebSocketCompleteOutstandingWork( |
There was a problem hiding this comment.
In general, I think we must be careful here about synchronization: Any completion handler and delegate callback could contain a call to the same websocket task, which may cause a deadlock if we are calling the handler while holding the task's mutex. Validate this carefully. Are there any such cases? Report.
1 participant
No description provided.