|
1.07 KB,
patch
|
jduell
:
review+
|
Details | Diff | Splinter Review |
WINNT 6.2 fx-team debug test mochitest-1 on 2014-09-24 04:17:09 PDT for push 251da0912168 slave: t-w864-ix-105 https://tbpl.mozilla.org/php/getParsedLog.php?id=48765823&tree=Fx-Team 04:32:07 WARNING - PROCESS-CRASH | /tests/content/base/test/test_websocket.html | application crashed [@ mozilla::LoadInfo::Release()] 04:32:07 INFO - Crash dump filename: c:\users\cltbld~1.t-w\appdata\local\temp\tmpxpxgxr.mozrunner\minidumps\9333801e-e84d-4988-843c-cb48a122b9b9.dmp 04:32:07 INFO - Operating system: Windows NT 04:32:07 INFO - 6.2.9200 04:32:07 INFO - CPU: x86 04:32:07 INFO - GenuineIntel family 6 model 30 stepping 5 04:32:07 INFO - 8 CPUs 04:32:07 INFO - Crash reason: EXCEPTION_BREAKPOINT 04:32:07 INFO - Crash address: 0x735d54ad 04:32:07 INFO - Thread 6 (crashed) 04:32:07 INFO - 0 xul.dll!mozilla::LoadInfo::Release() [LoadInfo.cpp:251da0912168 : 37 + 0x23] 04:32:07 INFO - eip = 0x735d54ad esp = 0x072ffba0 ebp = 0x072ffbac ebx = 0x0200f140 04:32:07 INFO - esi = 0x09fd7660 edi = 0x6fd21660 eax = 0x00000000 ecx = 0x6fdfff12 04:32:07 INFO - edx = 0x072fdf88 efl = 0x00000216 04:32:07 INFO - Found by: given as instruction pointer in context 04:32:07 INFO - 1 xul.dll!nsCOMPtr<nsILoadInfo>::~nsCOMPtr<nsILoadInfo>() [nsCOMPtr.h:251da0912168 : 522 + 0x5] 04:32:07 INFO - eip = 0x71cd6a54 esp = 0x072ffbb4 ebp = 0x072ffbe8 04:32:07 INFO - Found by: call frame info 04:32:07 INFO - 2 xul.dll!mozilla::net::BaseWebSocketChannel::~BaseWebSocketChannel() + 0x2a 04:32:07 INFO - eip = 0x71e55d46 esp = 0x072ffbc0 ebp = 0x072ffbe8 04:32:07 INFO - Found by: call frame info 04:32:07 INFO - 3 xul.dll!mozilla::net::WebSocketChannel::~WebSocketChannel() [WebSocketChannel.cpp:251da0912168 : 1105 + 0x121] 04:32:07 INFO - eip = 0x71e57463 esp = 0x072ffbc8 ebp = 0x072ffbe8 04:32:07 INFO - Found by: call frame info 04:32:07 INFO - 4 xul.dll!mozilla::net::WebSocketChannel::`scalar deleting destructor'(unsigned int) + 0xa 04:32:07 INFO - eip = 0x71e5845d esp = 0x072ffbf0 ebp = 0x072ffbf4 04:32:07 INFO - Found by: call frame info 04:32:07 INFO - 5 xul.dll!mozilla::net::WebSocketChannel::Release() [WebSocketChannel.cpp:251da0912168 : 88 + 0x6f] 04:32:07 INFO - eip = 0x71e57d12 esp = 0x072ffbfc ebp = 0x072ffc0c 04:32:07 INFO - Found by: call frame info 04:32:07 INFO - 6 xul.dll!nsCOMPtr<nsIInputStreamCallback>::assign_assuming_AddRef(nsIInputStreamCallback *) [nsCOMPtr.h:251da0912168 : 506 + 0x7] 04:32:07 INFO - eip = 0x71c7ae35 esp = 0x072ffc14 ebp = 0x072ffc20 04:32:07 INFO - Found by: call frame info 04:32:07 INFO - 7 xul.dll!nsInputStreamReadyEvent::Run() [nsStreamUtils.cpp:251da0912168 : 90 + 0x9] 04:32:07 INFO - eip = 0x71c89d63 esp = 0x072ffc28 ebp = 0x072ffc34 04:32:07 INFO - Found by: call frame info 04:32:07 INFO - 8 xul.dll!nsThread::ProcessNextEvent(bool,bool *) [nsThread.cpp:251da0912168 : 823 + 0xd] 04:32:07 INFO - eip = 0x71ca5215 esp = 0x072ffc3c ebp = 0x072ffd10 04:32:07 INFO - Found by: call frame info 04:32:07 INFO - 9 xul.dll!NS_ProcessNextEvent(nsIThread *,bool) [nsThreadUtils.cpp:251da0912168 : 265 + 0xc]
| Comment hidden (Treeherder Robot) |
|
||
Comment 2•3 years ago
|
||
Jason, this test is a crashy trainwreck (see also: bug 1031316, bug 1059042, and bug 1065747). Can we please either get somebody to work on fixing these problems or just disable this test?
| Comment hidden (Treeherder Robot) |
| Comment hidden (Treeherder Robot) |
|
||
Comment 5•3 years ago
|
||
Christoph, So this is the new mLoadInfo for websockets getting a crash when it's released. Got any ideas here? I notice nsLoadInfo does not use thread-safe reference counting--sounds like we may need to proxy the release to the main thread if we're not on it in the destructor?
|
|
||
Comment 6•3 years ago
|
||
Ryan, test_websocket contains almost all of our websocket tests (which is stupid design but a fact on the ground), so I'd rather chop it up than disable it. Also, the bugs you point out have less than a dozen TBPL comments among them--so are they crashing so much that they're high priority?
| Comment hidden (Treeherder Robot) |
|
(Assignee) | |
Updated•3 years ago
|
||
|
|
(Assignee) | |
Comment 8•3 years ago
|
||
Oh, I didn't want to clear my needinfo request, I was just about to add tanvi, haven't looked at the problem yet - will add my needinfo request back.
|
|
||
Comment 9•3 years ago
|
||
(In reply to Jason Duell (:jduell) from comment #6) > Ryan, > > test_websocket contains almost all of our websocket tests (which is stupid > design but a fact on the ground), so I'd rather chop it up than disable it. > Also, the bugs you point out have less than a dozen TBPL comments among > them--so are they crashing so much that they're high priority? I only mentioned the open crash bugs in that comment. test_websockets.html also has a long history of other intermittent failures a la bug 1026797. By my counts, 36 oranges have been filed against this test. Agreed that splitting up the test sounds like a good idea.
|
|
(Assignee) | |
Comment 10•3 years ago
|
||
(In reply to Jason Duell (:jduell) from comment #5) > Christoph, > > So this is the new mLoadInfo for websockets getting a crash when it's > released. Got any ideas here? Not at the moment, seems to be only an issue on WINNT, right? I notice nsLoadInfo does not use thread-safe > reference counting--sounds like we may need to proxy the release to the main > thread if we're not on it in the destructor? Possibly. I'll have a look at that.
| Comment hidden (Treeherder Robot) |
|
|
(Assignee) | |
Comment 12•3 years ago
|
||
Created attachment 8495282 [details] [diff] [review] bug_1072316_proxy_release_of_mloadinfo_on_websocket_channels.patch I am not super confident how to proxy the release, but presumably that patch is providing the missing part. Jason, what do you think?
|
|
||
Comment 13•3 years ago
|
||
Comment on attachment 8495282 [details] [diff] [review] bug_1072316_proxy_release_of_mloadinfo_on_websocket_channels.patch Review of attachment 8495282 [details] [diff] [review]: ----------------------------------------------------------------- Yes, this should be exactly right.
| Comment hidden (Treeherder Robot) |
|
|
||
Comment 15•3 years ago
|
||
Christoph, We're going to need this fix for HttpBaseChannel and nsBaseChannel too, and any other nsIChannel object that you added a mLoadInfo to. Channels are all generally thread-safe and can be released on any thread potentially. I'm agnostic about whether to do that in this bug or a new one. Sorry I missed this important detail when I did the reviews :)
|
|
(Assignee) | |
Comment 16•3 years ago
|
||
(In reply to Jason Duell (:jduell) from comment #15) > Christoph, > > We're going to need this fix for HttpBaseChannel and nsBaseChannel too, and > any other nsIChannel object that you added a mLoadInfo to. Channels are all > generally thread-safe and can be released on any thread potentially. > > I'm agnostic about whether to do that in this bug or a new one. I filed bug 1073282, where we can do that. > Sorry I missed this important detail when I did the reviews :) No problem - shouldn't be that hard to fix - thanks for bringing it up.
|
|
(Assignee) | |
Updated•3 years ago
|
||
| Comment hidden (Treeherder Robot) |
|
|
(Assignee) | |
Comment 18•3 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/01f3b20fd636
|
|
||
Comment 19•3 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/01f3b20fd636
|
|
||
Updated•3 years ago
|
||
| Comment hidden (Treeherder Robot) |
Description
•