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

Issue 872063 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocking:
issue 866708



Sign in to add a comment

RenderWidgetHostViewAuraTest leaks

Project Member Reported by roc...@chromium.org, Aug 7

Issue description

The failing tests are (in content_unittests):

  RenderWidgetHostViewAuraTest.ReturnedResources
  RenderWidgetHostViewAuraTest.TwoOutputSurfaces

See blocked bug for details and motivation regarding what's changing in Mojo. Likely the failures are caused by incorrect task ordering assumptions.

 
To clarify, failures were observed only on Linux ASAN bots, though they were not ASAN failures.
Cc: fsam...@chromium.org
+CC fsamuel as (AFAICT) the original author of the test, in case you have any insight or cycles to help look into this.

The problem here might not be with the actual test code, but could be in some production code whose side effects the tests depend on.
Labels: -Pri-3 Pri-1
And just to clarify, these failures appear when applying this CL[1]. The CL does not break any guarantees made by Mojo bindings, but it can affect the timing of arbitrary events. The failures therefore strongly imply that incorrect ordering assumptions being made somewhere, and this has been the case for all other blocking bugs which have been fixed so far.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/1145692
Cc: dtapu...@chromium.org roc...@chromium.org
Owner: ----
Status: Available (was: Started)
So these tests actually appear to have legit LSAN failures even on ToT. I don't know how it's possible that they aren't red all the time. They are blocking one of my CLs from landing, but I can repro locally (and consistently) with no patch applied.

Looks like leaks in MockWidgetInputHandler. Is anyone able to look at this?
Summary: RenderWidgetHostViewAuraTest leaks (was: RenderWidgetHostViewAuraTest failures with Mojo message dispatch changes)
Cc: nzolghadr@chromium.org
+nzolghadr@
Components: Blink>Input
rockot@ is this easily reproducible by applying the patch and building ASAN and running RenderWidgetHostViewAuraTest.ReturnedResources?
As I reported in comment #5, I actually seemed to be able to repro this without any patched applied. I can try again if you're not seeing the same.
Correct. I failed to see it locally. Maybe I'm missing something. What does your gn file look like? 
Nothing special:

  is_component_build = true
  is_debug = false
  enable_nacl = false
  dcheck_always_on = true
  is_asan = true
  use_goma = true
  # a bunch of definitely irrelevant stuff like GN path


Command line looks like:

  ASAN_OPTIONS=detect_leaks=1 out/asan/content_unittests --gtest_filter=RenderWidgetHostViewAuraTest.ReturnedResources

Fails every time for me on ToT. It is important that ASAN_OPTIONS environment be set accordingly since this is a leak.
I reduced down that file to the following:

#include "testing/gtest/include/gtest/gtest.h"


namespace content {

class RenderWidgetHostViewAuraTest : public testing::Test {
 public:
  RenderWidgetHostViewAuraTest() {
  }

};

TEST_F(RenderWidgetHostViewAuraTest, EmptyTest) {
}

}


And still running the following command tells me 3104 bytes leaked in 56 allocations.

autoninja -C out/asan content_unittests && ASAN_OPTIONS=detect_leaks=1 out/asan/content_unittests --gtest_filter=RenderWidgetHostViewAuraTest.EmptyTest

Am I missing something?
Not missing something, but this implies that it's likely the test runner's test suite (content::UnitTestTestSuite) which is responsible for the leak at least indirectly.

However, oddly enough, I can no longer repro on ToT as of this morninig. If you sync and try again, does this go away for you too? :/
Owner: nzolghadr@chromium.org
Status: Assigned (was: Available)
I still see this with ToT:

SUMMARY: AddressSanitizer: 3208 byte(s) leaked in 56 allocation(s).
[220611:220635:0917/133744.821303:1648140554108:ERROR:kill_posix.cc(84)] Unable to terminate process group 220636: No such process (3)
[1/1] RenderWidgetHostViewAuraTest.DestructionBeforeProperInitialization (21 ms)
1 test failed on exit:
    RenderWidgetHostViewAuraTest.DestructionBeforeProperInitialization (../../content/browser/renderer_host/render_widget_host_view_aura_unittest.cc:1088)
Tests took 1 seconds.


Apparently the number of bytes changed though after the rebase. Let me know what you want us to do with this bug.
Status: WontFix (was: Assigned)
I'm closing this as apparently it doesn't seem to be an issue with our test. Feel free to open it again if you have a way to reproducing this locally.
Owner: roc...@chromium.org
Status: Assigned (was: WontFix)
Re-opening because despite reproing locally even without my patch, apparently our ASAN bots only repro with https://chromium-review.googlesource.com/c/chromium/src/+/1145692 applied.
Owner: rockot@google.com
Cc: -roc...@chromium.org rockot@google.com
Project Member

Comment 19 by bugdroid1@chromium.org, Oct 23

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

commit 29297bc6f1d2624615ee412f0de1c31479a97cb1
Author: Ken Rockot <rockot@chromium.org>
Date: Tue Oct 23 00:30:49 2018

Fix potential leak in RWHV unit tests

MockWidgetInputHandler destruction may in some cases spin a RunLoop,
which in turn may allow certain Mojo objects to do work that (unsafely)
modifies the state of that same MockWidgetInputHandler. To prevent this
from happening, this CL explicitly closes the handler's Binding before
proceeding with the rest of the handler's destruction.

Bug:  872063 
Change-Id: I9c40455ac0182fbc435a0c2edfcf707aa18c94c8
Reviewed-on: https://chromium-review.googlesource.com/c/1294712
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#601800}
[modify] https://crrev.com/29297bc6f1d2624615ee412f0de1c31479a97cb1/content/test/mock_widget_input_handler.cc

Status: Fixed (was: Assigned)

Sign in to add a comment