1254829 - [e10s] WinXP opt reftest runs crash on shutdown [@ cLockHandle + 0x37]

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect, P1)

Description

[Ryan VanderMeulen [:RyanVM]]

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

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

WGL?

Comment 2

[Peter Chang[:pchang]]

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

Comment 3

[Peter Chang[:pchang]]

(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.

Comment 4

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=506279561849

Comment 5

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7c1352d5b788

Comment 6

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b138bd54b9c

Comment 7

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=419b5b3338f9

Comment 8

[Peter Chang[:pchang]]

Can't get the content process log from try server. Need to setup local environment to reproduce this issue.

Comment 9

[Peter Chang[:pchang]]

I'm checking the shutdown flow of content process, it looks like the deinitialization is not completed.

Comment 10

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3457f36db6a

Comment 11

[Peter Chang[:pchang]]

(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

Comment 12

[Peter Chang[:pchang]]

(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.

Comment 13

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3817c3e8ff4b

Comment 14

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=7098810e6a86

Comment 15

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ac128296c412

Comment 16

[Peter Chang[:pchang]]

Attachment: MozReview Request: Bug 1254829 - Change global context to static RefPtr to avoid destruction during QuickExit(), r?jrmuizel

Review commit: https://reviewboard.mozilla.org/r/42409/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/42409/

Comment 17

[Peter Chang[:pchang]]

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.

Comment 18

[Peter Chang[:pchang]]

(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

Comment 19

[Peter Chang[:pchang]]

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/

Comment 20

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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?

Comment 21

[Peter Chang[:pchang]]

(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

Comment 22

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc1855729214

Comment 23

[Peter Chang[:pchang]]

(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.

Comment 24

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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.

Comment 25

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

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

Comment 26

[Peter Chang[:pchang]]

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/

Comment 27

[Peter Chang[:pchang]]

(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().

Comment 28

[Jeff Muizelaar [:jrmuizel]]

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

Comment 29

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b90dc48988d4

Comment 30

[Benjamin Smedberg]

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?

Comment 31

[Jeff Muizelaar [:jrmuizel]]

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.

Comment 32

[Jeff Muizelaar [:jrmuizel]]

(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

[Pulsebot]

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/99e7d6fbcbd9

Comment 34

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=34bf1198e181

Comment 35

[Peter Chang[:pchang]]

(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.

Comment 36

[Peter Chang[:pchang]]

(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.

Comment 37

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 38

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cebceb52958e

Comment 39

[Peter Chang[:pchang]]

(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.

Comment 40

[Peter Chang[:pchang]]

https://reviewboard.mozilla.org/r/42409/#review41483

no the right way to fix the problem

Comment 41

[Peter Chang[:pchang]]

Attachment: MozReview Request: Bug 1254829 - Calling TerminateProcess for WindowsXP to bypass DLL detach handler, r?bsmedberg

Review commit: https://reviewboard.mozilla.org/r/44751/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/44751/

Comment 42

[Peter Chang[:pchang]]

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

Comment 43

[Milan Sreckovic [:milan] (needinfo for best results)]

(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.

Comment 44

[Ryan VanderMeulen [:RyanVM]]

(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

[Benjamin Smedberg]

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

Comment 46

[Peter Chang[:pchang]]

(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.

Comment 47

[Peter Chang[:pchang]]

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/

Comment 48

[Peter Chang[:pchang]]

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/

Comment 49

[Peter Chang[:pchang]]

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/

Comment 50

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1ad94f3446c

Comment 51

[Peter Chang[:pchang]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=68f63587e1b0

Comment 52

[Peter Chang[:pchang]]

https://reviewboard.mozilla.org/r/44751/#review41541

Good point, I will create another follow-up bug to unify QuickExit function.

Comment 53

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3819bbd16d8b

Comment 54

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3819bbd16d8b

Comment 55

[Ryan VanderMeulen [:RyanVM]]

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 :(

Comment 56

[Peter Chang[:pchang]]

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

Comment 57

[Peter Chang[:pchang]]

(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 58

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

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+

Comment 59

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0066805fc269