Design for safe destruction of net::WebSocketChannel

Adam Rice <ricea@chromium.org>

Thanks to Takeshi Yoshino <tyoshino@chromium.org> for advice and discussion

Objective

We need to be able to delete a net::WebSocketChannel object when it is no longer needed. However, we need to ensure that no accesses are made to the WebSocketChannel after it has been deleted. This is particularly difficult when it is deleted due to an exceptional condition such as a WebSocket protocol error or IPC error.

Background

WebSockets are initiated by renderers or WebWorkers. So the logical ownership of the WebSocket is with the child process.

In practice this means that the content::WebSocketDispatcherHost is the ultimate owner of all objects related to WebSockets for a particular process. The ownership graph looks like this:

As this diagram implies, when the WebSocketDispatcherHost deletes the content::WebSocketHost, everything else related to the socket will be cleanly and synchronously deleted.

Challenges

The difficulty arises for three reasons:

1. There is a loop in the call graph—WebSocketHost::EventInterface calls back into WebSocketDispatcherHost.
2. net::WebSocketChannel can receive several frames at once from net::WebSocketStream, and so needs to issue multiple events in a loop.
3. WebSocketDispatcherHost may need to delete the WebSocketHost object in response to an event.

WebSocketHost::EventInterface handles 5 types of events:

1. OnAddChannelResponse
2. OnDataFrame
3. OnFlowControl
4. OnClosingHandshake
5. OnDropChannel

Each of these translate 1:1 to an IPC which is sent to the renderer by WebSocketDispatcherHost. The channel must be deleted if the OnAddChannelResponse event has “fail” set to true, and when the OnDropChannel event occurs.

In addition, since any IPC can fail, it may be necessary to delete the channel in response to any of these events.

Proposed Design

The signature of all the event methods will be changed to return a value indicating whether the channel has been deleted, provisionally WebSocketEventInterface::CHANNEL_ALIVE or WebSocketEventInterface::CHANNEL_DELETED. Each method in WebSocketChannel which issues an event must check if the return value was CHANNEL_DELETED and return without doing any further processing if it was.

In some cases, the call-stack within WebSocketChannel contains several methods. Most notably ReadFrames() → OnReadDone() → ProcessFrame() → HandleFrame() → FailChannel(). All these methods must be modified so they can return CHANNEL_DELETED to their caller, passing CHANNEL_DELETED back up the call-stack.

The impact on the lower levels of the implementation is minor. Implementations of WebSocketStream need to be aware that they may be deleted while executing the callback that was passed to ReadFrames(). In practice this constraint does not appear problematic; net::WebSocketBasicStream already fulfills the requirement.

Alternative Designs Considered

1. Deleting WebSocketHost asynchronously by posting a task to the event loop. We did not adopt this approach because it makes reasoning about the timing and ordering of deletion difficult, and similar designs have caused us problems in the past.
2. Adding a reference count to WebSocketChannel and incrementing the reference count whenever we are inside a WebSocketChannel method. We did not adopt this design because it makes the timing of deletion ambiguous, and also makes the ownership of WebSocketChannel ambiguous.
3. Ignoring IPC errors. This would mean that only OnAddChannelResponse and OnDropChannel events could result in channel deletion, and WebSocketChannel could predict its own deletion accurately. We did not adopt this approach because it is not clear that ignoring IPC errors is safe, and because it would make refactoring of WebSocketChannel risky.
4. Posting all events to the EventInterface via the message pump. We did not adopt this approach because the risk of a serious performance penalty, particularly as an extra copy of each incoming message would almost certainly be needed.
5. Replacing the WebSocketEventInterface class with an int WaitForEvent(ScopedVector<Event>*, CompletionCallback) method on WebSocketChannel. This would remove the loop from the call graph so that it matched the ownership graph. We consider this a viable solution, but it is strange, complex and intrusive compared to our proposed design.

Revision History

• 2013-09-27: First version
• 2013-10-03: Made public