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

Issue 775563 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Oct 2017
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: ----
Type: ----



Sign in to add a comment

remoting_unittests failing on chromium.win/Win10 Tests x64

Project Member Reported by jonr...@chromium.org, Oct 17 2017

Issue description

remoting_unittests failing on chromium.win/Win10 Tests x64

Builders failed on: 
- Win10 Tests x64: 
  https://build.chromium.org/p/chromium.win/builders/Win10%20Tests%20x64

Example failure log: https://chromium-swarm.appspot.com/task?id=3943c6e548a6c710&refresh=10&show_raw=1

Each of the failures take the form:
[ RUN      ] HostAttributesTest.Sanity

Received fatal exception 0xc06d007e

Backtrace:

	RaiseException [0x00007FFB21C77788+68]

	__delayLoadHelper2 [0x00007FF635089C88+1b8] (f:\dd\vctools\delayimp\delayhlp.cpp:143)

	_tailMerge_MFPlat_DLL [0x00007FF634F6E905+3f]

	media::InitializeMediaFoundation [0x00007FF635041316+56]

	remoting::GetHostAttributes [0x00007FF634F284C2+312]

	remoting::HostAttributesTest_Sanity_Test::TestBody [0x00007FF633DF0BBC+2c]

	testing::internal::HandleExceptionsInMethodIfSupported<testing::Test,void> [0x00007FF633EF527D+3d]

	testing::Test::Run [0x00007FF633F01DBA+7a]

	testing::TestInfo::Run [0x00007FF633F01FB1+a1]

	testing::TestCase::Run [0x00007FF633F01E95+a5]

	testing::internal::UnitTestImpl::RunAllTests [0x00007FF633F02362+232]

	testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl,bool> [0x00007FF633EF531D+3d]

	testing::UnitTest::Run [0x00007FF633F020FB+db]

	base::TestSuite::Run [0x00007FF634544C9A+7a]

	base::LaunchUnitTests [0x00007FF6345423CB+45b]

	base::LaunchUnitTests [0x00007FF634541FEA+7a]

	main [0x00007FF633CF2259+161]

	__scrt_common_main_seh [0x00007FF634F73C09+11d] (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:11b)

	BaseThreadInitThunk [0x00007FFB22DD8364+14]

	RtlUserThreadStart [0x00007FFB254C5E91+21]

[503/888] HostAttributesTest.Sanity (CRASHED)

Unfortunately the range of potential causes is quite large: https://chromium.googlesource.com/chromium/src/+log/37f9056c80065ddc5efc6f4346deb3fd4907cb7a%5E..9680e8c3bca3e99adf3021fb3470863838924b85?pretty=fuller&n=

zijiehe@ you added the call from remoting to media::InitializeMediaFoundation could you take a look?


 
Labels: OS-Windows
Interesting, according to a very old bug, (https://bugs.chromium.org/p/chromium/issues/detail?id=339678), we should LoadLibrary(), i.e. MediaFoundationVideoEncodeAccelerator::PreSandboxInitialization() before calling MFStartup(), i.e. InitializeMediaFoundation() to avoid crashes within DelayLoad.

But unfortunately, the order of these two function calls are reversed in host_attributes.cc.

I am preparing a fix.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 17 2017

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

commit 282709e964c1b4e6cf7c0781a511d7c409dff9a2
Author: Zijie He <zijiehe@chromium.org>
Date: Tue Oct 17 22:11:39 2017

[Chromoting] Avoid crashing when media foundation is not present on the system

According to a very old bug,
(https://bugs.chromium.org/p/chromium/issues/detail?id=339678),
we should LoadLibrary(), i.e.
MediaFoundationVideoEncodeAccelerator::PreSandboxInitialization() before calling
MFStartup(), i.e. InitializeMediaFoundation() to avoid crashes within DelayLoad.

See the bug for details.

Bug:  chromium:775563 
Change-Id: I7c54569304b8806095a32b009bed5bec287e91f1
Reviewed-on: https://chromium-review.googlesource.com/723774
Commit-Queue: Zijie He <zijiehe@chromium.org>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509563}
[modify] https://crrev.com/282709e964c1b4e6cf7c0781a511d7c409dff9a2/remoting/host/host_attributes.cc

I cannot reproduce the issue on my machine, so I cannot ensure that this change fixes the crash. Let me keep an eye on it.
Status: Fixed (was: Assigned)
The issue has been resolved. See https://build.chromium.org/p/chromium.win/builders/Win10%20Tests%20x64?numbuilds=200.
Labels: Merge-Request-63
Change in #c2 needs to be merged to M63. This change impacts only Chromoting.
Project Member

Comment 6 by sheriffbot@chromium.org, Oct 31 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Is the change listed at #2 well baked/verified in Canary and fully safe to merge to M63?
This change impacts only Chromoting (//remoting folder), so we do not verify it on Canary. The change itself is safe to be merged to M63.
Labels: -Merge-Review-63 Merge-Approved-63
Approving merge to M63 branch 3239 based on comment #8. Please merge ASAP. Thank you.
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 1 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0fe11510494cfa1e731dcf1cff70a82e94131c55

commit 0fe11510494cfa1e731dcf1cff70a82e94131c55
Author: Zijie He <zijiehe@chromium.org>
Date: Wed Nov 01 17:35:44 2017

[Chromoting] Avoid crashing when media foundation is not present on the system

According to a very old bug,
(https://bugs.chromium.org/p/chromium/issues/detail?id=339678),
we should LoadLibrary(), i.e.
MediaFoundationVideoEncodeAccelerator::PreSandboxInitialization() before calling
MFStartup(), i.e. InitializeMediaFoundation() to avoid crashes within DelayLoad.

See the bug for details.

Bug:  chromium:775563 
Change-Id: I7c54569304b8806095a32b009bed5bec287e91f1
Reviewed-on: https://chromium-review.googlesource.com/723774
Commit-Queue: Zijie He <zijiehe@chromium.org>
Reviewed-by: Lambros Lambrou <lambroslambrou@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#509563}(cherry picked from commit 282709e964c1b4e6cf7c0781a511d7c409dff9a2)
Reviewed-on: https://chromium-review.googlesource.com/748686
Reviewed-by: Zijie He <zijiehe@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#333}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/0fe11510494cfa1e731dcf1cff70a82e94131c55/remoting/host/host_attributes.cc

Merged. Thank you.

Sign in to add a comment