Closed
Bug 1254829
Opened 8 years ago
Closed 8 years ago
[e10s] WinXP opt reftest runs crash on shutdown [@ cLockHandle + 0x37]
Categories
(Core :: Graphics: CanvasWebGL, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: RyanVM, Assigned: pchang)
References
(Blocks 1 open bug)
Details
(Keywords: crash, Whiteboard: [e10s-orangeblockers])
Attachments
(1 file, 1 obsolete file)
58 bytes,
text/x-review-board-request
|
benjamin
:
review+
ritu
:
approval-mozilla-aurora+
|
Details |
https://treeherder.mozilla.org/logviewer.html#?job_id=17687071&repo=try 17:19:21 INFO - REFTEST SUITE-END | Shutdown 17:19:21 INFO - REFTEST INFO | Slowest test took 5001ms (file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/bugs/966992-1.html) 17:19:21 INFO - REFTEST INFO | Total canvas count = 2 17:19:22 INFO - JavaScript error: chrome://reftest/content/reftest.js, line 2008: NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIPropertyBag2.getPropertyAsAString] 17:19:22 INFO - REFTEST INFO | Downloading symbols from: https://queue.taskcluster.net/v1/task/b54ffPeRTEOZ4_wpoG01rQ/artifacts/public/build/firefox-47.0a1.en-US.win32.crashreporter-symbols.zip 17:19:28 INFO - REFTEST INFO | Copy/paste: C:\slave\test\build\win32-minidump_stackwalk.exe c:\docume~1\cltbld~1.t-x\locals~1\temp\tmpclalf_.mozrunner\minidumps\a37922d0-a51b-4813-a76e-4324526943b2.dmp c:\docume~1\cltbld~1.t-x\locals~1\temp\tmp47phkx 17:19:36 INFO - REFTEST INFO | Saved minidump as C:\slave\test\build\blobber_upload_dir\a37922d0-a51b-4813-a76e-4324526943b2.dmp 17:19:36 INFO - REFTEST INFO | Saved app info as C:\slave\test\build\blobber_upload_dir\a37922d0-a51b-4813-a76e-4324526943b2.extra 17:19:36 ERROR - REFTEST PROCESS-CRASH | Main app process exited normally | application crashed [@ cLockHandle + 0x37] 17:19:36 INFO - Crash dump filename: c:\docume~1\cltbld~1.t-x\locals~1\temp\tmpclalf_.mozrunner\minidumps\a37922d0-a51b-4813-a76e-4324526943b2.dmp 17:19:36 INFO - Operating system: Windows NT 17:19:36 INFO - 5.1.2600 Service Pack 3 17:19:36 INFO - CPU: x86 17:19:36 INFO - GenuineIntel family 6 model 30 stepping 5 17:19:36 INFO - 8 CPUs 17:19:36 INFO - Crash reason: EXCEPTION_ACCESS_VIOLATION_READ 17:19:36 INFO - Crash address: 0x7ff30007 17:19:36 INFO - Assertion: Unknown assertion type 0x00000000 17:19:36 INFO - Process uptime: 1205 seconds 17:19:36 INFO - Thread 0 (crashed) 17:19:36 INFO - 0 opengl32.dll!cLockHandle + 0x37 17:19:36 INFO - eip = 0x5ed195ec esp = 0x0012f56c ebp = 0x0012f57c ebx = 0x7ff30000 17:19:36 INFO - esi = 0x00000000 edi = 0x00000000 eax = 0x00000000 ecx = 0x7ffde000 17:19:36 INFO - edx = 0x5edac6e0 efl = 0x00010297 17:19:36 INFO - Found by: given as instruction pointer in context 17:19:36 INFO - 1 opengl32.dll!wglDeleteContext + 0x1e 17:19:36 INFO - eip = 0x5ed1bb3a esp = 0x0012f584 ebp = 0x0012f58c ebx = 0x00010000 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 2 xul.dll!mozilla::gl::GLContextWGL::~GLContextWGL() [GLContextProviderWGL.cpp:4dc8809c6357 : 311 + 0xc] 17:19:36 INFO - eip = 0x105b64c3 esp = 0x0012f594 ebp = 0x0012f5a4 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 3 xul.dll!mozilla::gl::GLContextWGL::`scalar deleting destructor'(unsigned int) + 0xb 17:19:36 INFO - eip = 0x105b6878 esp = 0x0012f5a0 ebp = 0x0012f5a4 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 4 xul.dll!mozilla::detail::GenericRefCounted<0>::Release() [GenericRefCounted.h:4dc8809c6357 : 96 + 0xa] 17:19:36 INFO - eip = 0x105b3e2a esp = 0x0012f5ac ebp = 0x0012f5c8 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 5 xul.dll!_CRT_INIT [crtdll.c : 414 + 0x2] 17:19:36 INFO - eip = 0x11c6ede3 esp = 0x0012f5b4 ebp = 0x0012f5c8 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 6 xul.dll!__DllMainCRTStartup [crtdll.c : 522 + 0xc] 17:19:36 INFO - eip = 0x11c6f00b esp = 0x0012f5d0 ebp = 0x0012f60c 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 7 xul.dll!_DllMainCRTStartup [crtdll.c : 472 + 0xe] 17:19:36 INFO - eip = 0x11c6ef35 esp = 0x0012f614 ebp = 0x0012f620 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 8 ntdll.dll!LdrpCallInitRoutine + 0x14 17:19:36 INFO - eip = 0x7c90118a esp = 0x0012f628 ebp = 0x0012f640 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 9 ntdll.dll!RtlDestroyEnvironment + 0x178 17:19:36 INFO - eip = 0x7c923aba esp = 0x0012f648 ebp = 0x0012f6c4 17:19:36 INFO - Found by: previous frame's frame pointer 17:19:36 INFO - 10 kernel32.dll!_ExitProcess + 0x42 17:19:36 INFO - eip = 0x7c81ca96 esp = 0x0012f6cc ebp = 0x0012f7b8 17:19:36 INFO - Found by: previous frame's frame pointer 17:19:36 INFO - 11 kernel32.dll!ExitProcess + 0x14 17:19:36 INFO - eip = 0x7c81cb0e esp = 0x0012f7c0 ebp = 0x0012f7cc 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 12 msvcr120.dll!__crtExitProcess [crt0dat.c : 774 + 0x9] 17:19:36 INFO - eip = 0x00f43fac esp = 0x0012f7d4 ebp = 0x0012f7d8 17:19:36 INFO - Found by: previous frame's frame pointer 17:19:36 INFO - 13 msvcr120.dll!doexit [crt0dat.c : 678 + 0x8] 17:19:36 INFO - eip = 0x00f4427e esp = 0x0012f7e0 ebp = 0x0012f820 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 14 msvcr120.dll!_exit [crt0dat.c : 433 + 0xc] 17:19:36 INFO - eip = 0x00f8bbc7 esp = 0x0012f828 ebp = 0x0012f834 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 15 xul.dll!mozilla::dom::ContentChild::ActorDestroy(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason) [ContentChild.cpp:4dc8809c6357 : 2243 + 0x8] 17:19:36 INFO - eip = 0x10e6766c esp = 0x0012f83c ebp = 0x0012f840 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 16 xul.dll!mozilla::dom::PContentChild::DestroySubtree(mozilla::ipc::IProtocolManager<mozilla::ipc::IProtocol>::ActorDestroyReason) [PContentChild.cpp:4dc8809c6357 : 9039 + 0xd] 17:19:36 INFO - eip = 0x103b9e9f esp = 0x0012f848 ebp = 0x0012f870 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 17 xul.dll!mozilla::dom::PContentChild::OnChannelClose() [PContentChild.cpp:4dc8809c6357 : 8645 + 0x9] 17:19:36 INFO - eip = 0x103bc596 esp = 0x0012f878 ebp = 0x0012f8a0 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 18 xul.dll!mozilla::ipc::MessageChannel::NotifyChannelClosed() [MessageChannel.cpp:4dc8809c6357 : 2022 + 0xb] 17:19:36 INFO - eip = 0x10298455 esp = 0x0012f884 ebp = 0x0012f8a0 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 19 xul.dll!mozilla::ipc::MessageChannel::OnNotifyMaybeChannelError() [MessageChannel.cpp:4dc8809c6357 : 1898 + 0x7] 17:19:36 INFO - eip = 0x10298f3a esp = 0x0012f88c ebp = 0x0012f8a0 17:19:36 INFO - Found by: call frame info 17:19:36 INFO - 20 xul.dll!MessageLoop::RunTask(Task *) [message_loop.cc:4dc8809c6357 : 364 + 0x11] 17:19:36 INFO - eip = 0x10289aa7 esp = 0x0012f8a8 ebp = 0x0012f8b0 17:19:36 INFO - Found by: call frame info
Flags: needinfo?(jgilbert)
WGL?
Assignee | ||
Comment 2•8 years ago
|
||
There are two static objects, gGlobalContext and sWGLLib in GLContextProviderWGL.cpp. I suspected gGlobalContext was not clear in [1]. Therefore, it is possible that sWGLLib got destroyed before gGlobalContext and it caused this crash problem. [1]https://dxr.mozilla.org/mozilla-central/source/gfx/gl/GLContextProviderWGL.cpp#723
Assignee | ||
Comment 3•8 years ago
|
||
(In reply to peter chang[:pchang][:peter] from comment #2) > There are two static objects, gGlobalContext and sWGLLib in > GLContextProviderWGL.cpp. > I suspected gGlobalContext was not clear in [1]. Therefore, it is possible > that sWGLLib got destroyed before gGlobalContext and it caused this crash > problem. > > [1]https://dxr.mozilla.org/mozilla-central/source/gfx/gl/ > GLContextProviderWGL.cpp#723 I just confirmed that ContentChild::ActorDestroy was called before GLContextProviderWGL::Shutdown when I close content tab in e10s mode. Take this bug to provide patches.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → howareyou322
Reporter | ||
Updated•8 years ago
|
Whiteboard: [e10s-orangeblockers]
Assignee | ||
Comment 4•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=506279561849
Assignee | ||
Comment 5•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c1352d5b788
Assignee | ||
Comment 6•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b138bd54b9c
Assignee | ||
Comment 7•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=419b5b3338f9
Assignee | ||
Comment 8•8 years ago
|
||
Can't get the content process log from try server. Need to setup local environment to reproduce this issue.
Assignee | ||
Comment 9•8 years ago
|
||
I'm checking the shutdown flow of content process, it looks like the deinitialization is not completed.
Assignee | ||
Comment 10•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3457f36db6a
Assignee | ||
Comment 11•8 years ago
|
||
(In reply to peter chang[:pchang][:peter] from comment #10) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3457f36db6a I just confirmed the content process with opt build didn't go through gfxPlatform::Shutdown flow because early return in [1]. I guess this behavior is due to performance concern. Without calling gfxPlatform::Shutdown, it might cause problem when destroy these two static object, gGlobalContext[2] and sWGLLib[3], because gGlobalContext is calling on sWGLLib in deconstructor. [1]https://dxr.mozilla.org/mozilla-central/source/dom/ipc/ContentChild.cpp#2248 [2]https://dxr.mozilla.org/mozilla-central/source/gfx/gl/GLContextProviderWGL.cpp?from=GLContextProviderWGL.cpp#690 [3]https://dxr.mozilla.org/mozilla-central/source/gfx/gl/GLContextProviderWGL.cpp?from=GLContextProviderWGL.cpp#25
Assignee | ||
Comment 12•8 years ago
|
||
(In reply to peter chang[:pchang][:peter] from comment #11) > Without calling gfxPlatform::Shutdown, it might cause problem when destroy > these two static object, gGlobalContext[2] and sWGLLib[3], because > gGlobalContext is calling on sWGLLib in deconstructor. I mean destructor here.
Assignee | ||
Comment 13•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3817c3e8ff4b
Updated•8 years ago
|
Priority: -- → P1
Assignee | ||
Comment 14•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7098810e6a86
Assignee | ||
Comment 15•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac128296c412
Assignee | ||
Comment 16•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42409/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42409/
Assignee | ||
Comment 17•8 years ago
|
||
Comment on attachment 8734638 [details] MozReview Request: Bug 1254829 - Change global context to static RefPtr to avoid destruction during QuickExit(), r?jrmuizel As I mentioned in comment 11, I think we still need to run the shutdown flow for e10s content process to avoid unexpected condition happened.
Attachment #8734638 -
Flags: review?(benjamin)
Assignee | ||
Comment 18•8 years ago
|
||
(In reply to peter chang[:pchang][:peter] from comment #11) > (In reply to peter chang[:pchang][:peter] from comment #10) > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3457f36db6a > > I just confirmed the content process with opt build didn't go through > gfxPlatform::Shutdown flow because early return in [1]. I guess this > behavior is due to performance concern. > > Without calling gfxPlatform::Shutdown, it might cause problem when destroy > these two static object, gGlobalContext[2] and sWGLLib[3], because > gGlobalContext is calling on sWGLLib in deconstructor. I will create another bug to fix the dependency bteween these two objects. > > [1]https://dxr.mozilla.org/mozilla-central/source/dom/ipc/ContentChild. > cpp#2248 > [2]https://dxr.mozilla.org/mozilla-central/source/gfx/gl/ > GLContextProviderWGL.cpp?from=GLContextProviderWGL.cpp#690 > [3]https://dxr.mozilla.org/mozilla-central/source/gfx/gl/ > GLContextProviderWGL.cpp?from=GLContextProviderWGL.cpp#25
Assignee | ||
Comment 19•8 years ago
|
||
Comment on attachment 8734638 [details] MozReview Request: Bug 1254829 - Change global context to static RefPtr to avoid destruction during QuickExit(), r?jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42409/diff/1-2/
Attachment #8734638 -
Attachment description: MozReview Request: Bug 1254829 - Run XPCOM shutdown for e10s content process → MozReview Request: Bug 1254829 - Run XPCOM shutdown for e10s content process, r?billm
Attachment #8734638 -
Flags: review?(benjamin) → review?(wmccloskey)
I'd really rather avoid the whole shutdown process here since it shouldn't be necessary. QuickExit calls _exit, which isn't supposed to run any destructors or anything. Maybe it's not working the way we expect on Windows?
Assignee | ||
Comment 21•8 years ago
|
||
(In reply to Bill McCloskey (:billm) from comment #20) > I'd really rather avoid the whole shutdown process here since it shouldn't > be necessary. QuickExit calls _exit, which isn't supposed to run any > destructors or anything. Maybe it's not working the way we expect on Windows? Bill, exit will perform object destruction of static and global members[1], but abort[2] won't. Before e10s, we did run shutdown process for chrome. Why it is unnecessary to call shutdown for e10s content process? I think some modules, like gfx, might need the shutdown callback to do some clean up. [1]http://www.cplusplus.com/reference/cstdlib/exit/ [2]http://www.cplusplus.com/reference/cstdlib/abort/?kw=abort
Assignee | ||
Updated•8 years ago
|
Flags: needinfo?(wmccloskey)
Assignee | ||
Comment 22•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc1855729214
Assignee | ||
Comment 23•8 years ago
|
||
(In reply to peter chang[:pchang][:peter] from comment #22) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc1855729214 I just upload another patch which tried to register the global context cleanup callback via atexit to see the crash still happens or not.
We call _exit, not exit. exit will invoke destructors. _exit will not. There's no reason to shut down content processes because they don't need to write anything to disk. That's the only part of the shutdown process that's actually useful to the user (and the only part that's externally visible). In debug builds, we also run shutdown to check for leaks, but that's not necessary in opt builds.
Flags: needinfo?(wmccloskey)
Attachment #8734638 -
Flags: review?(wmccloskey)
Comment on attachment 8734638 [details] MozReview Request: Bug 1254829 - Change global context to static RefPtr to avoid destruction during QuickExit(), r?jrmuizel https://reviewboard.mozilla.org/r/42409/#review40151
Assignee | ||
Comment 26•8 years ago
|
||
Comment on attachment 8734638 [details] MozReview Request: Bug 1254829 - Change global context to static RefPtr to avoid destruction during QuickExit(), r?jrmuizel Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42409/diff/2-3/
Attachment #8734638 -
Attachment description: MozReview Request: Bug 1254829 - Run XPCOM shutdown for e10s content process, r?billm → MozReview Request: Bug 1254829 - Change global context to static RefPtr to avoid destruction during QuickExit(), r?jrmuizel
Attachment #8734638 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 27•8 years ago
|
||
(In reply to Bill McCloskey (:billm) from comment #24) > We call _exit, not exit. exit will invoke destructors. _exit will not. > > There's no reason to shut down content processes because they don't need to > write anything to disk. That's the only part of the shutdown process that's > actually useful to the user (and the only part that's externally visible). > In debug builds, we also run shutdown to check for leaks, but that's not > necessary in opt builds. Thanks for your information. (In reply to peter chang[:pchang][:peter] from comment #26) > Comment on attachment 8734638 [details] > MozReview Request: Bug 1254829 - Change global context to static RefPtr to > avoid destruction during QuickExit(), r?jrmuizel > > Review request updated; see interdiff: > https://reviewboard.mozilla.org/r/42409/diff/2-3/ I just found that this issue was fixed by changing global context from RefPtr to StaticRefPtr to avoid destruction during QuickExit().
Updated•8 years ago
|
Attachment #8734638 -
Flags: review?(jmuizelaar) → review+
Comment 28•8 years ago
|
||
Comment on attachment 8734638 [details] MozReview Request: Bug 1254829 - Change global context to static RefPtr to avoid destruction during QuickExit(), r?jrmuizel https://reviewboard.mozilla.org/r/42409/#review40343
Reporter | ||
Updated•8 years ago
|
Flags: needinfo?(jgilbert)
Comment 30•8 years ago
|
||
I am now kinda worried. RefPtr destructors should not be running during QuickExit (really, no destructor-like code should be running). The stack indicates we're again seeing some the old bugs where _exit() on Windows is running dll-detach handlers which end up running c++ destructors. This is a pretty bad bandaid: I suspect we should instead change QuickExit to do something different on Windows, such as call TerminateProcess. Jeff, do you agree?
Flags: needinfo?(jmuizelaar)
Comment 31•8 years ago
|
||
Comment on attachment 8734638 [details] MozReview Request: Bug 1254829 - Change global context to static RefPtr to avoid destruction during QuickExit(), r?jrmuizel Yeah, sorry this is fixing this the wrong way. Please back it out.
Flags: needinfo?(jmuizelaar)
Attachment #8734638 -
Flags: review+ → review-
Comment 32•8 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #30) > I am now kinda worried. RefPtr destructors should not be running during > QuickExit (really, no destructor-like code should be running). The stack > indicates we're again seeing some the old bugs where _exit() on Windows is > running dll-detach handlers which end up running c++ destructors. > > This is a pretty bad bandaid: I suspect we should instead change QuickExit > to do something different on Windows, such as call TerminateProcess. > > Jeff, do you agree? Yes, that sounds reasonable.
Comment 33•8 years ago
|
||
Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/99e7d6fbcbd9
Assignee | ||
Comment 34•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=34bf1198e181
Assignee | ||
Comment 35•8 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #32) > (In reply to Benjamin Smedberg [:bsmedberg] from comment #30) > > I am now kinda worried. RefPtr destructors should not be running during > > QuickExit (really, no destructor-like code should be running). The stack > > indicates we're again seeing some the old bugs where _exit() on Windows is > > running dll-detach handlers which end up running c++ destructors. > > > > This is a pretty bad bandaid: I suspect we should instead change QuickExit > > to do something different on Windows, such as call TerminateProcess. > > > > Jeff, do you agree? > > Yes, that sounds reasonable. Agree, I just uploaded another patch and wait for another try run.(In reply to Jeff Muizelaar [:jrmuizel] from comment #31) > Comment on attachment 8734638 [details] > MozReview Request: Bug 1254829 - Change global context to static RefPtr to > avoid destruction during QuickExit(), r?jrmuizel > > Yeah, sorry this is fixing this the wrong way. Please back it out. I will create another bug to land this patch because it is better to use StaticRefPtr for global variables.
Assignee | ||
Comment 36•8 years ago
|
||
(In reply to peter chang[:pchang][:peter] from comment #34) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=34bf1198e181 The reftest only ran for 3 minutes, it was quite strange.
Reporter | ||
Comment 37•8 years ago
|
||
From the log:
> 07:52:54 INFO - runreftest.py: error: unrecognized arguments: --e10s
There was a recent change which made e10s the default for various test suite and in turn broke the patch we've been using for force-enable it. I haven't managed to work out the right incantation yet for the new config :(. I'll keep trying and post a link to a working patch once I have one.
Assignee | ||
Comment 38•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cebceb52958e
Assignee | ||
Comment 39•8 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM] from comment #37) > From the log: > > 07:52:54 INFO - runreftest.py: error: unrecognized arguments: --e10s > There was a recent change which made e10s the default for various test suite > and in turn broke the patch we've been using for force-enable it. I haven't > managed to work out the right incantation yet for the new config :(. I'll > keep trying and post a link to a working patch once I have one. I see, then I will request review for my patch first. Thanks for the info.
Assignee | ||
Comment 40•8 years ago
|
||
https://reviewboard.mozilla.org/r/42409/#review41483 no the right way to fix the problem
Assignee | ||
Updated•8 years ago
|
Attachment #8734638 -
Attachment is obsolete: true
Assignee | ||
Comment 41•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/44751/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/44751/
Attachment #8738897 -
Flags: review?(benjamin)
Assignee | ||
Comment 42•8 years ago
|
||
https://reviewboard.mozilla.org/r/44751/#review41487 ::: dom/ipc/ContentChild.cpp:2321 (Diff revision 1) > void > ContentChild::QuickExit() > { > NS_WARNING("content process _exit()ing"); > + > +#ifdef XP_WIN32 So far this issue was happend on WindowXP only, therefore, I only change the behavior in window 32 bit platform
(In reply to peter chang[:pchang][:peter] from comment #42) >... > > So far this issue was happend on WindowXP only, therefore, I only change the > behavior in window 32 bit platform Are we worried about Windows XP 64-bit users? Don't know off hand how many of those exist.
Reporter | ||
Comment 44•8 years ago
|
||
(In reply to peter chang[:pchang][:peter] from comment #39) > I see, then I will request review for my patch first. Thanks for the info. Here's an updated patch you can use to force e10s on. https://hg.mozilla.org/try/rev/c2362c49372a81e99216f7df7c97ecf0838c34a3
Comment 45•8 years ago
|
||
Comment on attachment 8738897 [details] MozReview Request: Bug 1254829 - Calling TerminateProcess for WindowsXP to bypass DLL detach handler, r?bsmedberg https://reviewboard.mozilla.org/r/44751/#review41541 I'll take this as-is, but there is also a PluginModuleChild::QuickExit function which could use the same fix. It would be great to have a unified QuickExit function. r=me in any case
Attachment #8738897 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 46•8 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #43) > (In reply to peter chang[:pchang][:peter] from comment #42) > >... > > > > So far this issue was happend on WindowXP only, therefore, I only change the > > behavior in window 32 bit platform > > Are we worried about Windows XP 64-bit users? Don't know off hand how many > of those exist. I think it might have similar problem about Window 64 platform on some day. I will apply my patch for Window 32/64.
Assignee | ||
Comment 47•8 years ago
|
||
Comment on attachment 8738897 [details] MozReview Request: Bug 1254829 - Calling TerminateProcess for WindowsXP to bypass DLL detach handler, r?bsmedberg Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44751/diff/1-2/
Assignee | ||
Comment 48•8 years ago
|
||
Comment on attachment 8738897 [details] MozReview Request: Bug 1254829 - Calling TerminateProcess for WindowsXP to bypass DLL detach handler, r?bsmedberg Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44751/diff/2-3/
Assignee | ||
Comment 49•8 years ago
|
||
Comment on attachment 8738897 [details] MozReview Request: Bug 1254829 - Calling TerminateProcess for WindowsXP to bypass DLL detach handler, r?bsmedberg Review request updated; see interdiff: https://reviewboard.mozilla.org/r/44751/diff/3-4/
Assignee | ||
Comment 50•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1ad94f3446c
Assignee | ||
Comment 51•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=68f63587e1b0
Assignee | ||
Comment 52•8 years ago
|
||
https://reviewboard.mozilla.org/r/44751/#review41541 Good point, I will create another follow-up bug to unify QuickExit function.
Reporter | ||
Comment 54•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3819bbd16d8b
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
Reporter | ||
Comment 55•8 years ago
|
||
Looks good on Ash! Please request Aurora approval on this when you get a chance. That said, it unfortunately didn't help with bug 1227718 and related, so removing that link :(
Flags: needinfo?(howareyou322)
See Also: 1227718 →
Reporter | ||
Updated•8 years ago
|
status-firefox47:
--- → affected
Assignee | ||
Comment 56•8 years ago
|
||
Comment on attachment 8738897 [details] MozReview Request: Bug 1254829 - Calling TerminateProcess for WindowsXP to bypass DLL detach handler, r?bsmedberg Approval Request Comment [Feature/regressing bug #]: e10s [User impact if declined]: it might cause crash with Widonws opt build during shutdown [Describe test coverage new/current, TreeHerder]: Pass Reftest in TreeHerder [Risks and why]: low risk because we did less than previous behavior during shutdown [String/UUID change made/needed]: none
Flags: needinfo?(howareyou322)
Attachment #8738897 -
Flags: approval-mozilla-aurora?
Assignee | ||
Comment 57•8 years ago
|
||
(In reply to Benjamin Smedberg [:bsmedberg] from comment #45) > Comment on attachment 8738897 [details] > MozReview Request: Bug 1254829 - Calling TerminateProcess for WindowsXP to > bypass DLL detach handler, r?bsmedberg > > https://reviewboard.mozilla.org/r/44751/#review41541 > > I'll take this as-is, but there is also a PluginModuleChild::QuickExit > function which could use the same fix. It would be great to have a unified > QuickExit function. > r=me in any case Create bug 1263499 to unify the QuickExit function.
Comment on attachment 8738897 [details] MozReview Request: Bug 1254829 - Calling TerminateProcess for WindowsXP to bypass DLL detach handler, r?bsmedberg Fixes test failure in e10s mode, Aurora47+
Attachment #8738897 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 59•8 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-aurora/rev/0066805fc269
You need to log in
before you can comment on or make changes to this bug.
Description
•