RenderWidgetHostViewAuraTest leaks |
|||||||||||||
Issue descriptionThe 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.
,
Aug 8
+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.
,
Aug 8
,
Aug 8
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
,
Aug 16
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?
,
Aug 16
,
Aug 20
+nzolghadr@
,
Aug 28
rockot@ is this easily reproducible by applying the patch and building ASAN and running RenderWidgetHostViewAuraTest.ReturnedResources?
,
Aug 28
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.
,
Aug 29
Correct. I failed to see it locally. Maybe I'm missing something. What does your gn file look like?
,
Aug 30
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.
,
Sep 17
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?
,
Sep 17
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? :/
,
Sep 17
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.
,
Oct 3
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.
,
Oct 16
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.
,
Oct 17
,
Oct 17
,
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
,
Oct 23
|
|||||||||||||
►
Sign in to add a comment |
|||||||||||||
Comment 1 by roc...@chromium.org
, Aug 7