This issue is a security regression. If you are not able to fix this quickly, please revert the change that introduced it.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

enne: Would you mind taking a look or suggesting another owner?

Findit result:

Author: enne
Project: chromium
Changelist: https://chromium.googlesource.com/chromium/src/+/1a1031cf083156956a7b6f4934089281da6981c0
Time: Tue Sep 06 23:11:20 2016
File gpu_process_transport_factory.cc is changed in this cl (and is part of stack frame #2, "content::GpuProcessTransportFactory::GpuProcessTransportFactory")
File gpu_process_transport_factory.cc is changed in this cl (and is part of stack frame #2, "content::GpuProcessTransportFactory::~GpuProcessTransportFactory")
Minimum distance from crash line to modified line: 6. (file: gpu_process_transport_factory.cc, crashed on: 182, modified: 176).

Suspected Project: chromium
Suspected Component: Internals>Core

Sure I'll take a look.  I think this was https://codereview.chromium.org/2297473002 for sure.  I already fixed a similar issue in https://codereview.chromium.org/2318753002.

I tried to repro but idk what I'm doing.

This looks like it's a layout test? From the config it's using:
app_args = --enable-logging --allow-file-access-from-files --disable-gesture-requirement-for-media-playback --use-gl=any --disable-gl-drawing-for-tests --run-layout-test --dump-render-tree

I tried putting it in my LayoutTests dir and running it on windows, it just times out.

I tried running content_shell myself manually but idk how to run it that way, I just get a "#READY" until I ^C. Running without the flags and exiting content_shell does not hit the DCHECK either.

Are you super sure it's that DCHECK that would get hit?

I tried following https://docs.google.com/document/d/15ZqPDlz3a8Ac08uWWBc-tM4IZzhP-kYEuQAXiJRiH24/edit# to run the clusterfuzz tool myself fully. But the instructions do not work at all in windows.

I got as far as figuring out how to install pip myself. Then to get it in the path and run "pip install pyyaml" so the run_gestures_on_device_local.py runs. Then I figured out "pip install mozprocess" to get that, as the import silently fails but running it with the --build and --config errors with undefined mozprocess... but that actually didn't help, I get this:

C:\Users\danakj>python D:\src\clusterfuzz\src\tools\on_demand\run_gestures_on_device_local.py --config C:\users\danakj\Downloads\config_5441870908096512.zip --build D:\src\c\src\out\Debug
D:\src\c\src\out\Debug\content_shell.exe --no-sandbox --user-data-dir=c:\users\danakj\appdata\local\temp\tmpgw9uel\temp\user_profile_0 --log-net-log=c:\users\danakj\appdata\local\temp\tmpgw9uel\temp\net_log_0 --enable-logging --allow-file-access-from-files --disable-gesture-requirement-for-media-playback --use-gl=any --disable-gl-drawing-for-tests --run-layout-test --dump-render-tree http://127.0.0.1:8000/er-common-data-bundles/WebKit/LayoutTests/inspector/heap/fuzz-http-8.html
Starting run number: 0
Traceback (most recent call last):
  File "D:\src\clusterfuzz\src\tools\on_demand\run_gestures_on_device_local.py", line 526, in <module>
    main()
  File "D:\src\clusterfuzz\src\tools\on_demand\run_gestures_on_device_local.py", line 512, in main
    runner.run()
  File "D:\src\clusterfuzz\src\tools\on_demand\run_gestures_on_device_local.py", line 129, in run
    timeout=self.test_timeout)
  File "D:\src\clusterfuzz\src\common\process_handler.py", line 147, in runProcess
    process_output = mozprocess.processhandler.StoreOutput()
NameError: global name 'mozprocess' is not defined

+inferno for windows wisdom.

I also tried running the command line that the above prints out without success. It just sits there after this:

C:\Users\danakj>D:\src\c\src\out\Debug\content_shell.exe --no-sandbox --user-data-dir=c:\users\danakj\appdata\local\temp\tmpgw9uel\temp\user_profile_0 --log-net-log=c:\users\danakj\appdata\local\temp\tmpgw9uel\temp\net_log_0 --enable-logging --allow-file-access-from-files --disable-gesture-requirement-for-media-playback --use-gl=any --disable-gl-drawing-for-tests --run-layout-test --dump-render-tree C:\users\danakj\Downloads\fuzz-http-8.html

C:\Users\danakj>[16560:18060:0908/180150:167430062:ERROR:shell_net_log.cc(65)] Could not open file c:\users\danakj\appdata\local\temp\tmpgw9uel\temp\net_log_0 for net logging
#READY
err: rx::Renderer11::initializeD3DDevice(752): Failed creating Debug D3D11 device - falling back to release runtime.

Hm, also there's clearly no gpu going on its falling back to software compositing. That doesn't match what happens on my machine of course, IDK why the clusterfuzz windows is acting so differently there, but I'd have to change the command line to get that with --disable-gpu, but ofc it just hangs with that all the same, and doesn't crash when exiting non-layout-test mode since this seems tied to layout_test_browser_main.cc

Also when I run the html fuzz file in run-webkit-tests, it just times out, dcheck does not happen. Same thing if I pass --additional-driver-flag=--disable-gpu

danakj: re comment #7.  I am confident the DCHECK in ~GpuProcessTransportFactory would be hit.  From the callstack, it looks like ~PerCompositorData is being hit, which means that some PerCompositorData is still in the map and it is not empty.  I can't imagine another way for this to be hit from the GPTF destructor.

I think we can work around this, but it's frustrating to not know how we got into this weird state.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/15ca791b6abd8cabac85497ea6290b276cb38152

commit 15ca791b6abd8cabac85497ea6290b276cb38152
Author: enne <enne@chromium.org>
Date: Fri Sep 09 17:41:03 2016

Fix use-after-free in ~GpuProcessTransportFactory

Previously, this used to leak, but now that it gets destroyed during
shutdown, this DCHECK turns into a security issue.  Work around this
by clearing data in the correct order even if it should already be
cleared prior to destruction.

R=danakj@chromium.org
BUG= 645075 

Review-Url: https://codereview.chromium.org/2317363004
Cr-Commit-Position: refs/heads/master@{#417620}

[modify] https://crrev.com/15ca791b6abd8cabac85497ea6290b276cb38152/content/browser/compositor/gpu_process_transport_factory.h

I tried running the command from #10 with an asan build, but because it just hangs it doesn't repro anything of course..

I also ran run-webkit-tests with the asan build and it times out as before and doesn't crash.

third_party/WebKit/Tools/Scripts/run-webkit-tests --additional-driver-flag=--disable-gpu --build-directory=out_asan dana
Using port 'win-win10'
Test configuration: <win10, x86, release>
View the test results at file://D:\src\c\src\out_asan\Release\layout-test-results/results.html
View the archived results dashboard at file://D:\src\c\src\out_asan\Release\layout-test-results/dashboard.html
Baseline search path: disable-gpu -> win -> generic
Using Release build
Pixel tests enabled
Regular timeout: 6000, slow test timeout: 30000
Command line: out_asan\Release\content_shell.exe --disable-gpu --run-layout-test --enable-direct-write --enable-crash-reporter --crash-dumps-dir=out_asan\Release\crash-dumps -

Found 1 test; running 1, skipping 0.

Ruby is not installed; can't generate pretty patches.

Running 1 content_shell.

[1/1] dana/fuzz-http-8.html failed unexpectedly (test timed out)

0 tests ran as expected, 1 didn't:


Regressions: Unexpected timeouts (1)
  dana/fuzz-http-8.html [ Timeout ]

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2dd865b0da80e59a150b505a07d2fda05aa62d56

commit 2dd865b0da80e59a150b505a07d2fda05aa62d56
Author: enne <enne@chromium.org>
Date: Mon Sep 12 19:35:55 2016

Turn ~GpuProcessTransportFactory DCHECK into a CHECK

If this DCHECK fails it means that some ui::Compositor has not been
destroyed and unregistered itself.  This is potentially really bad
as ui::Compositor assumes that its context factory outlives it.

This is to try to track down why this could be happening by
making it a larger failure than it has been in the past.

BUG= 645075 

Review-Url: https://codereview.chromium.org/2323303003
Cr-Commit-Position: refs/heads/master@{#418003}

[modify] https://crrev.com/2dd865b0da80e59a150b505a07d2fda05aa62d56/content/browser/compositor/gpu_process_transport_factory.cc

645912 implies this isn't layout-test-specific.

Users experienced this crash on the following builds:

Win Dev 55.0.2853.0 -  0.44 CPM, 173 reports, 34 clients (signature content::SoftwareOutputDeviceWin::~SoftwareOutputDeviceWin)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

https://codereview.chromium.org/2317363004 landed after 55.0.2853.0, so it's not unexpected to still see it there.

Reusing this bug as sadrul linked crash/ to it.

The CHECK turned up a content shell issue ( issue 646633 ) which I have a patch up for, but it appears that this is happening in full browser builds too: https://crash.corp.google.com/browse?q=custom_data.ChromeCrashProto.ptype%3D%27browser%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27content%3A%3AGpuProcessTransportFactory%3A%3A~GpuProcessTransportFactory%27%20AND%20product.name%3D%27Chrome%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D

I'm hoping that there will be some additional repro or fuzzer test that triggers this, as I am unable to figure out how we're leaking ui::Compositors in practice.

Is it possible that some of the chrome windows (i.e. views::Widgets, which would have their own ui::Compositor instances) are still up and showing when this destruction code runs?

That's one good theory though we have no idea which it would be, we tried a few.

Users experienced this crash on the following builds:

Win Dev 55.0.2859.0 -  0.70 CPM, 112 reports, 99 clients (signature content::GpuProcessTransportFactory::~GpuProcessTransportFactory)
Mac Canary 55.0.2861.0 -  0.72 CPM, 1 reports, 1 clients (signature content::GpuProcessTransportFactory::~GpuProcessTransportFactory)

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

/cc+ ikilpatrick@ who is hitting this crash from some CL. Not sure if related to this or not.

A browser (and not content shell layout test) repro for this would be excellent if you have one, ikilpatrick.

Friendly ping, this a stable blocker for M54, please try to have a fix in by the first week of October so it can be fixed in time for the release.

I don't understand why this was marked as M54.  It's not in M54. 

To sum up: I first introduced a unique_ptr in the first patch, which meant that compositors no longer leaked and shutdown, which caused a use after free due to a bad variable ordering.  I fixed that in the second patch.  Then, I turned a DCHECK which should have caught leaking compositors into a CHECK to find more occurences of the original bug in #3.  This turned up a crash which I fixed in #4.

https://codereview.chromium.org/2297473002 use unique_ptr (415192)
https://codereview.chromium.org/2317363004 fix use-after-free (417620)
https://codereview.chromium.org/2323303003 DCHECK -> CHECK (418003)
https://codereview.chromium.org/2340693003 fix leaking compositor in layout tests (421591)

I was leaving this open to try to see if the CHECK caught any more users who had repro steps.  I'll just close this until somebody finds the CHECK again as it's clearly more noise than help.  :)

This is top#1 browser crash on latest Chrome Dev Channel i.e., 55.0.2883.11 on Windows currently there are 182 crashes from 116 unique clients, Please find the link for the crashes below :


https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20AND%20product.version%3D%2755.0.2883.11%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27browser%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27content%3A%3AGpuProcessTransportFactory%3A%3A~GpuProcessTransportFactory%27&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D

Reverting here: https://codereview.chromium.org/2340083002.  Will request merge to m55 when that lands.

Thank you enne@. 

Marking as "ReleaseBlock-Dev" for tomorrow's M55 dev release.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7c5d5c0654053d852ded6de612e7debc4073358a

commit 7c5d5c0654053d852ded6de612e7debc4073358a
Author: enne <enne@chromium.org>
Date: Mon Oct 17 20:28:22 2016

Revert of Turn ~GpuProcessTransportFactory DCHECK into a CHECK (patchset #1 id:1 of https://codereview.chromium.org/2323303003/ )

Reason for revert:
Causing shutdown crashes and can't find repro yet.

Original issue's description:
> Turn ~GpuProcessTransportFactory DCHECK into a CHECK
>
> If this DCHECK fails it means that some ui::Compositor has not been
> destroyed and unregistered itself.  This is potentially really bad
> as ui::Compositor assumes that its context factory outlives it.
>
> This is to try to track down why this could be happening by
> making it a larger failure than it has been in the past.
>
> BUG= 645075 
>
> Committed: https://crrev.com/2dd865b0da80e59a150b505a07d2fda05aa62d56
> Cr-Commit-Position: refs/heads/master@{#418003}

TBR=danakj@chromium.org
BUG= 645075 

Review-Url: https://codereview.chromium.org/2340083002
Cr-Commit-Position: refs/heads/master@{#425767}

[modify] https://crrev.com/7c5d5c0654053d852ded6de612e7debc4073358a/content/browser/compositor/gpu_process_transport_factory.cc

Please merge to M55 branch 2883 ASAP. Thank you.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/205e226a491bf7d0e0fd4fad04bf0b051538ab65

commit 205e226a491bf7d0e0fd4fad04bf0b051538ab65
Author: Adrienne Walker <enne@chromium.org>
Date: Mon Oct 17 20:45:24 2016

Revert of Turn ~GpuProcessTransportFactory DCHECK into a CHECK (patchset #1 id:1 of https://codereview.chromium.org/2323303003/ )

Reason for revert:
Causing shutdown crashes and can't find repro yet.

Original issue's description:
> Turn ~GpuProcessTransportFactory DCHECK into a CHECK
>
> If this DCHECK fails it means that some ui::Compositor has not been
> destroyed and unregistered itself.  This is potentially really bad
> as ui::Compositor assumes that its context factory outlives it.
>
> This is to try to track down why this could be happening by
> making it a larger failure than it has been in the past.
>
> BUG= 645075 
>
> Committed: https://crrev.com/2dd865b0da80e59a150b505a07d2fda05aa62d56
> Cr-Commit-Position: refs/heads/master@{#418003}

TBR=danakj@chromium.org
BUG= 645075 

Review-Url: https://codereview.chromium.org/2340083002
Cr-Commit-Position: refs/heads/master@{#425767}
(cherry picked from commit 7c5d5c0654053d852ded6de612e7debc4073358a)

Review URL: https://codereview.chromium.org/2426573003 .

Cr-Commit-Position: refs/branch-heads/2883@{#159}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/205e226a491bf7d0e0fd4fad04bf0b051538ab65/content/browser/compositor/gpu_process_transport_factory.cc

Revert is already merged to M55 at comment #40. So removing "ReleaseBlock-Dev" label.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/205e226a491bf7d0e0fd4fad04bf0b051538ab65

commit 205e226a491bf7d0e0fd4fad04bf0b051538ab65
Author: Adrienne Walker <enne@chromium.org>
Date: Mon Oct 17 20:45:24 2016

Revert of Turn ~GpuProcessTransportFactory DCHECK into a CHECK (patchset #1 id:1 of https://codereview.chromium.org/2323303003/ )

Reason for revert:
Causing shutdown crashes and can't find repro yet.

Original issue's description:
> Turn ~GpuProcessTransportFactory DCHECK into a CHECK
>
> If this DCHECK fails it means that some ui::Compositor has not been
> destroyed and unregistered itself.  This is potentially really bad
> as ui::Compositor assumes that its context factory outlives it.
>
> This is to try to track down why this could be happening by
> making it a larger failure than it has been in the past.
>
> BUG= 645075 
>
> Committed: https://crrev.com/2dd865b0da80e59a150b505a07d2fda05aa62d56
> Cr-Commit-Position: refs/heads/master@{#418003}

TBR=danakj@chromium.org
BUG= 645075 

Review-Url: https://codereview.chromium.org/2340083002
Cr-Commit-Position: refs/heads/master@{#425767}
(cherry picked from commit 7c5d5c0654053d852ded6de612e7debc4073358a)

Review URL: https://codereview.chromium.org/2426573003 .

Cr-Commit-Position: refs/branch-heads/2883@{#159}
Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768}

[modify] https://crrev.com/205e226a491bf7d0e0fd4fad04bf0b051538ab65/content/browser/compositor/gpu_process_transport_factory.cc

[Automated comment] removing mislabelled merge-merged-2840

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot