New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 837950 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Check failed: sink_map_.empty()

Project Member Reported by noel@chromium.org, Apr 28 2018

Issue description

viz frame_sink_manager_impl.cc sometimes hits this CHECK running browser tests, causing a flaky failure, even though the test has PASSED.

Example: VideoPlayerBrowserTest.OpenSingleVideoOnDownloads (CRASHED) ASAN
  http://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20(1)/builds/27201

Build log:

[28281:28281:0426/011811.065102:INFO:CONSOLE(0)] "Error in event handler for runtime.onSuspend: TypeError: The first argument is the receiver and must be an object", source: chrome-extension://dmboannefpncccogfdikhmhpmdnddgoe/_generated_background_page.html (0)
[28281:28281:0426/011811.583009:INFO:CONSOLE(0)] "[SUCCESS] [openSingleVideoOnDownloads]", source: chrome-extension://ljoplibgfehghmibaoaepfagnmbbfiga/_generated_background_page.html (0)
[28281:28451:0426/011814.490053:WARNING:discardable_shared_memory_manager.cc(431)] Some MojoDiscardableSharedMemoryManagerImpls are still alive. They will be leaked.
[28281:28281:0426/011814.883053:FATAL:frame_sink_manager_impl.cc(58)] Check failed: sink_map_.empty(). 
#0 0x00000097ad31 (/b/s/w/ir/out/Release/browser_tests+0x97ad30)
#1 0x00000f235bac (/b/s/w/ir/out/Release/browser_tests+0xf235bab)
...
[557/824] VideoPlayerBrowserTest.OpenSingleVideoOnDownloads (CRASHED)

 

Comment 1 by noel@chromium.org, Apr 28 2018

Cc: fsam...@chromium.org
Owner: kylec...@chromium.org
Status: Assigned (was: Untriaged)
Log of VideoPlayerBrowserTest.OpenSingleVideoOnDownloads from that ASAN browser test build attached.
asan-viz-check-crash.log.txt
13.9 KB View Download

Comment 2 by noel@chromium.org, Apr 28 2018

Components: Platform>Apps>FileManager

Comment 3 by noel@chromium.org, May 1 2018

Another example running Providers/FileManagerBrowserTest.Test/0

http://build.chromium.org/p/chromium.memory/builders/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20(1)/builds/27258

[9994:9994:0430/051629.043985:INFO:CONSOLE(0)] "[SUCCESS] [requestMount]", source: chrome-extension://oobinhbdbiehknkpbpejbbpdbkdjmoco/_generated_background_page.html (0)
[9994:10078:0430/051629.419079:WARNING:audio_sync_reader.cc(193)] AudioSyncReader::Read timed out, audio glitch count=10
[9994:10078:0430/051630.694042:WARNING:audio_sync_reader.cc(193)] AudioSyncReader::Read timed out, audio glitch count=20
[9994:10078:0430/051630.893032:WARNING:audio_sync_reader.cc(175)] ASR: No room in socket buffer.: Broken pipe (32)
[9994:9994:0430/051631.206320:FATAL:frame_sink_manager_impl.cc(58)] Check failed: sink_map_.empty(). 
#0 0x00000096aaa1 (/b/s/w/ir/out/Release/browser_tests+0x96aaa0)
#1 0x00000f401cdc (/b/s/w/ir/out/Release/browser_tests+0xf401cdb)

Comment 4 by noel@chromium.org, May 2 2018

Labels: CrOSFilesCategory-Testing

Comment 5 by noel@chromium.org, May 7 2018

Cc: slangley@chromium.org
Ahem, another one: VideoPlayerBrowserTest.OpenSingleVideoOnDrive

https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium.memory%2FLinux_Chromium_OS_ASan_LSan_Tests__1_%2F27335%2F%2B%2Frecipes%2Fsteps%2Fbrowser_tests%2F0%2Fstdout

[24768:24768:0505/182636.394177:INFO:CONSOLE(0)] "[SUCCESS] [openSingleVideoOnDrive]", source: chrome-extension://ljoplibgfehghmibaoaepfagnmbbfiga/_generated_background_page.html (0)
[24768:24768:0505/182639.953595:WARNING:display.cc(450)] VSync for last SwapBuffers is not received!
[24768:24768:0505/182640.840833:FATAL:frame_sink_manager_impl.cc(58)] Check failed: sink_map_.empty(). 
#0 0x00000096bff1 (/b/s/w/ir/out/Release/browser_tests+0x96bff0)
#1 0x00000f4d6c8c (/b/s/w/ir/out/Release/browser_tests+0xf4d6c8b)
Labels: -Pri-3 Pri-1
I just hit this locally, repro steps is "open and close some Youtube tabs", sorry if this isn't very helpful :). The crash was during shutdown.

[236119:236119:0507/104219.719345:ERROR:frame_sink_provider_impl.cc(34)] No RenderWidgetHost exists with id 1 in process 15
[236119:236119:0507/104219.719579:ERROR:frame_sink_provider_impl.cc(51)] No RenderWidgetHost exists with id 1 in process 15
[236119:236119:0507/104223.166893:ERROR:frame_sink_provider_impl.cc(34)] No RenderWidgetHost exists with id 4 in process 18
[236119:236119:0507/104223.167208:ERROR:frame_sink_provider_impl.cc(51)] No RenderWidgetHost exists with id 4 in process 18
[236119:236119:0507/104223.167445:ERROR:frame_sink_provider_impl.cc(34)] No RenderWidgetHost exists with id 1 in process 18
[236119:236119:0507/104223.167707:ERROR:frame_sink_provider_impl.cc(51)] No RenderWidgetHost exists with id 1 in process 18
[236119:236119:0507/104223.283204:ERROR:frame_sink_provider_impl.cc(34)] No RenderWidgetHost exists with id 1 in process 16
[236119:236119:0507/104223.283558:ERROR:frame_sink_provider_impl.cc(51)] No RenderWidgetHost exists with id 1 in process 16
[236119:236119:0507/104223.386977:ERROR:media_internals.cc(103)] Cannot get RenderProcessHost
[236119:236119:0507/104225.347836:ERROR:frame_sink_provider_impl.cc(34)] No RenderWidgetHost exists with id 1 in process 17
[236119:236119:0507/104225.348201:ERROR:frame_sink_provider_impl.cc(51)] No RenderWidgetHost exists with id 1 in process 17
[236119:236119:0507/104227.882342:FATAL:frame_sink_manager_impl.cc(58)] Check failed: sink_map_.empty(). 
#0 0x7f8cff649cfd base::debug::StackTrace::StackTrace()
#1 0x7f8cff3744bc base::debug::StackTrace::StackTrace()
#2 0x7f8cff3e5fba logging::LogMessage::~LogMessage()
#3 0x7f8cde740828 viz::FrameSinkManagerImpl::~FrameSinkManagerImpl()
#4 0x7f8cde740be9 viz::FrameSinkManagerImpl::~FrameSinkManagerImpl()
#5 0x7f8cf8e7a366 content::BrowserMainLoop::ShutdownThreadsAndCleanUp()
#6 0x7f8cf8e8123b content::BrowserMainRunnerImpl::Shutdown()
#7 0x7f8cf8e6d1df content::BrowserMain()
#8 0x7f8cfab99c07 content::RunNamedProcessTypeMain()
#9 0x7f8cfab9d754 content::ContentMainRunnerImpl::Run()
#10 0x7f8cfab90915 content::ContentServiceManagerMainDelegate::RunEmbedderProcess()
#11 0x7f8cffbc5dbc service_manager::Main()
#12 0x7f8cfab96b25 content::ContentMain()
#13 0x5568661cd246 ChromeMain
#14 0x5568661cd152 main
#15 0x7f8cdef412b1 __libc_start_main
#16 0x5568661cd02a _start
Cc: lethalantidote@chromium.org
Sorry, I haven't had much time to look into this yet. The crash happened in browser_tests so there is a CompositorFrameSinkImpl that wasn't destroyed. The only things that create a CompositorFrameSinkImpl is offscreen canvas (which isn't used in these tests) or if the UseSurfaceLayerForVideo experiment is enabled (which I thought it wasn't). I'll dig more tomorrow.
Oh, yep a VideoFrameResourceProvider and CompositorFrameSinkImpl are created when I run VideoPlayerBrowserTest.OpenSingleVideoOnDrive.
Cc: mlamouri@chromium.org
mlamouri, is cc::Surface for video turned on right now?
No but it is part of fieldtrial_testing_config.json which means it is picked up by browser tests in the waterfall.

Comment 11 Deleted

Comment 14 by noel@chromium.org, May 11 2018

Components: Tests>Flaky
And as of right now [1], we see KeyboardOperations/FileManagerBrowserTest.Test/3 saw #13 flake once.

[1] https://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=KeyboardOperations%2FFileManagerBrowserTest.Test%2F3

I've dug into "KeyboardOperations/FileManagerBrowserTest.Test/3 " a bit more. There are multiple RenderProcessHostImpls that get created but never destroyed by the test. The test is using UseSurfaceLayerForVideo and I guess there are video frames somewhere so that creates EmbeddedFrameSinkImpl in the browser. The test does destroy the EmbeddedFrameSinkImpl but I guess there is a race in shutdown and sometimes that hasn't happened yet.

We destroy all EmbeddedFrameSinkImpl for a RenderProcessHostImpl when it's destroyed, but since that isn't happening that safe guard is also missing. I'll look why RenderProcessHostImpls aren't getting destroyed.

It also seems like there are some races in video code during tests that are contributing to this and likely other problems.
I'm also seeing this with the new mojo app version of touch hud for ash.

Use "xinput list" to get your mouse pointer id for "touch devices".

* out/Default/chrome --ash-host-window-bounds="0+800-1024x768" --user-data-dir=/w/udd --ash-dev-shortcuts --ash-debug-shortcuts --touch-devices=9 --show-taps-app
* (wait a minute, then click a few times to show touch points)
* Ctrl-Shift-Q twice to exit

Same stack as #6
Project Member

Comment 17 by bugdroid1@chromium.org, May 17 2018

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

commit f51b238ae0f790112770f94915c208a67064d5ca
Author: kylechar <kylechar@chromium.org>
Date: Thu May 17 16:52:47 2018

Clear FrameSinkManagerImpl sink_map_.

The browser process doesn't shut down cleanly enough to ensure that
everything in |sink_map_| has been destroyed first. This is causing some
issues with test flake, so remove DCHECK and explicitly clear
|sink_map_|.

Bug:  837950 
Change-Id: I260219fa57476c1adc749b3b1d748220c9582e2e
Reviewed-on: https://chromium-review.googlesource.com/1062628
Reviewed-by: Saman Sami <samans@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559569}
[modify] https://crrev.com/f51b238ae0f790112770f94915c208a67064d5ca/components/viz/service/frame_sinks/frame_sink_manager_impl.cc

Project Member

Comment 18 by bugdroid1@chromium.org, May 17 2018

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

commit 0a989d8e1c1365ae45e4eff2e779f322e9cd3a08
Author: kylechar <kylechar@chromium.org>
Date: Thu May 17 21:08:56 2018

Cleanup EmbeddedFrameSinkProviderImpl.

EmbededFrameSinkProviderImpl was being destroyed when
RenderProcessHostImpl was destroyed, which is too late and doesn't
necessarily happen before shutdown. Destroy the provider earlier along
with all embedded CompositorFrameSinks.

Bug:  837950 
Change-Id: I38695e54338c57488789a0f2357a57b04cd05e7e
Reviewed-on: https://chromium-review.googlesource.com/1064799
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559678}
[modify] https://crrev.com/0a989d8e1c1365ae45e4eff2e779f322e9cd3a08/content/browser/renderer_host/render_process_host_impl.cc

Project Member

Comment 19 by bugdroid1@chromium.org, May 17 2018

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

commit dcd85713e45c49454a09609f07912bdf9a6b2270
Author: kylechar <kylechar@chromium.org>
Date: Thu May 17 21:19:06 2018

Revert "Clear FrameSinkManagerImpl sink_map_."

This reverts commit f51b238ae0f790112770f94915c208a67064d5ca.

Reason for revert: Not needed, https://crrev.com/c/1064799 should solve the test flake in a better way.

Original change's description:
> Clear FrameSinkManagerImpl sink_map_.
> 
> The browser process doesn't shut down cleanly enough to ensure that
> everything in |sink_map_| has been destroyed first. This is causing some
> issues with test flake, so remove DCHECK and explicitly clear
> |sink_map_|.
> 
> Bug:  837950 
> Change-Id: I260219fa57476c1adc749b3b1d748220c9582e2e
> Reviewed-on: https://chromium-review.googlesource.com/1062628
> Reviewed-by: Saman Sami <samans@chromium.org>
> Commit-Queue: kylechar <kylechar@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#559569}

TBR=kylechar@chromium.org,samans@chromium.org

Change-Id: Ieeeb1ed23227320254282fcff4dad098aec785b8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  837950 
Reviewed-on: https://chromium-review.googlesource.com/1064772
Reviewed-by: kylechar <kylechar@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559680}
[modify] https://crrev.com/dcd85713e45c49454a09609f07912bdf9a6b2270/components/viz/service/frame_sinks/frame_sink_manager_impl.cc

Comment 20 by noel@chromium.org, May 18 2018

Thanks kylechar@.  Please note on  https://crbug.com/843030#c18  that I renamed the FileManagerBrowserTests to give them more meaningful names.  You'd be looking for

   KeyboardOperations/FilesAppBrowserTest.Test*

for #15 now, k.



   
Status: Fixed (was: Assigned)
Should be fixed. I don't see that DCHECK being hit in browser_tests on "Linux Chromium OS ASan LSan Tests (1)" since yesterday.

Sign in to add a comment