58655 - nsSyncHelper uses a native event queue

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect, P3)

Description

[Dan Mosedale (:dmosedale, :dmose)]

According to dougt, this should be a monitored event queue.

Comment 1

[Darin Fisher]

I'm just wondering... do we have an instance where this is getting us into
trouble?

Comment 2

[Doug Turner (:dougt)]

It would be a hard thing to identify user level problems caused by this bug in http.

gagan, this is another place where messages could become out of sync.

Comment 3

[Darin Fisher]

Understood... can you point me to an example that uses monitored event queues.
I'd like understand what exactly needs to change.

Comment 4

[Darin Fisher]

Doug, is this just as simple as replacing CreateThreadEventQueue() with
CreateMonitoredThreadEventQueue()?
--darin

Comment 5

[Doug Turner (:dougt)]

yes.  and lots of testing.  :-)

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

This seems potentially fairly dangerous; nominating for mozilla0.9.

Comment 7

[Darin Fisher]

In the grand scheme of things nsSyncHelper is going away in favor of a true
synchronous HTTP implementation.  My feeling on this bug is therefore to just
leave it for now, and wait for the better solution.  However, I'd like to know
if people feel that this should still be "fixed" at least for the time being.
AFAIK, synchronous HTTP is heavily used (at least not in mozilla).

Comment 8

[Darin Fisher]

I meant to say that synchronous HTTP is _NOT_ heavily used in mozilla.

Comment 9

[Darin Fisher]

ok.. it looks like the new sync http impl is not going to happen for a while, so
i'm going to go ahead and patch up the existing impl.

adding dependency on bug 59434.

Comment 10

[Darin Fisher]

nominating for moz 0.9

Comment 11

[Darin Fisher]

Attachment: Makes HTTP use a monitored event queue inside OpenInputStream

Comment 12

[Doug Turner (:dougt)]

looks good.  I am shocked that this didn't cause more problems.  r=doug.

please ci after I land my branch/.

Comment 13

[Darin Fisher]

only causes problems if there are other threads touching HTTP.

Comment 14

[Scott MacGregor]

sr=mscott

Comment 15

[Darin Fisher]

Fix checked in.

Comment 16

[Tom Everingham]

verified