1007610 - Transport collects unsendable messages

Status: RESOLVED FIXED

(DevTools :: Debugger, defect)

Description

[[e-mail]]

Steps to reproduce problem:
1. Start a debuggable application
2. Connect the remote debugger
3. Disconnect the remote debugger

At this point, the transport's stream is closed. However this does not prevent actors from attempting to send packets such as cycle collector console messages. The transport collects these in its outgoing string, and attempts to write them to its output. As soon as an error occurs the streams are closed, however no other action is taken. Closed streams will still wait successfully, so further packets accumulate in the output stream, but the onOutputStreamReady callback will simply fail to write each time.

The accumulation of the string causes XPConnect to convert more and more data to bytes each time the write is attempted, and in an unoptimised build this results in main-thread jank once the outgoing string reaches a megabyte or so.

I was pointed to bug 797639 which rewrites the way the transport sends packets and this includes a number of additional features which appear to reduce if not prevent this problem, such as flushing the outgoing queue after an error.

Comment 1

[Jim Blandy :jimb]

Where are these packets getting queued up, exactly?

Comment 2

[[e-mail]]

(In reply to Jim Blandy from comment #1)
> Where are these packets getting queued up, exactly?

http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/transport.js#94

_outgoing only gets truncated in the event of a successful write.

Comment 3

[Jim Blandy :jimb]

Okay, right. So the issue is that enqueuing a packet to a closed transport doesn't even throw. It certainly should.

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Yeah, bug 797639 will improve this, since it's manipulating an array at least.  But in this bug we could make a further change to modify the send methods to stop queuing after close.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Neil, now that bug 797639 has landed, are you still seeing this issue?

Comment 6

[[e-mail]]

I'm trying to come up with STR, I think setting javascript.options.mem.log will generate a steady stream of messages but the client end gives me a connection timeout (I don't know if this relates to the TypeError: this.window is null message) and I'm not sure if this will trigger the behaviour.

Comment 7

[[e-mail]]

Since bug 797638 landed I have been unable to reproduce this issue in any way that I can think of, except actually doing any genuine browser debugging (since right now I have nothing to debug).

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Alright, let's assume this was fixed by bug 797639.  We can always reopen later if there's evidence otherwise.