New issue
Advanced search Search tips

Issue 891718 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 11
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: ----



Sign in to add a comment

WaylandScreenTest.MultipleOutputsAddedAndRemoved failing on chromium.memory/Linux Chromium OS ASan LSan Tests (1)

Project Member Reported by sheriff-...@appspot.gserviceaccount.com, Oct 3

Issue description

Filed by sheriff-o-matic@appspot.gserviceaccount.com on behalf of smcgruer@google.com

WaylandScreenTest.MultipleOutputsAddedAndRemoved failing on chromium.memory/Linux Chromium OS ASan LSan Tests (1)

Builders failed on: 
- Linux Chromium OS ASan LSan Tests (1): 
  https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29

Failed 5/5 since the CL landed. Example failure:

  ==19794==ERROR: AddressSanitizer: stack-use-after-return on address 0x7f4b07a3f368 at pc 0x000000618c6e bp 0x7f4b0439f4a0 sp 0x7f4b0439f498
  WRITE of size 8 at 0x7f4b07a3f368 thread T9 (fake_wayland_se)
      #0 0x618c6d in wl::ServerObject::OnResourceDestroyed(wl_resource*) ui/ozone/platform/wayland/fake_server.cc:838:18
      #1 0x37380ef in destroy_resource third_party/wayland/src/src/wayland-server.c:677:3
      #2 0x36ff3e9 in for_each_helper third_party/wayland/src/src/wayland-util.c:374:10

...

 
Cc: -smcgruer@google.com
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 3

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

commit b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Wed Oct 03 15:32:53 2018

Revert "[ozone/wayland] Add remove output functionality"

This reverts commit 14de2307d2980ecb5282767d3bef6bd568ad282b.

Reason for revert: Newly added test WaylandScreenTest.MultipleOutputsAddedAndRemoved fails on ChromeOS ASAN/LSAN; see  crbug.com/891718 

Original change's description:
> [ozone/wayland] Add remove output functionality
> 
> This CL adds a new functionality to the existing WaylandOutputManager
> and WaylandScreen classes. Now, it is possible to remove and
> change primary displays on hot plugs and removes.
> 
> To test that functionality, a new test has also been added.
> 
> The logic that assigns output ids has also been changed.
> Now, we are using numeric names that a Wayland compositor
> announces during Global and GlobalRemove calls. That is,
> each global object is assigned an unique numeric name, which can
> be freely used as an id.
> 
> Bug:  890276 , 875161
> Change-Id: I775bdd40a946c9f0c5dde89eba6e36c4615e1f21
> Reviewed-on: https://chromium-review.googlesource.com/c/1254146
> Commit-Queue: Maksim Sisov <msisov@igalia.com>
> Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#596155}

TBR=rjkroege@chromium.org,msisov@igalia.com

Change-Id: I8ae1f0fa1df71b464096abd0fcd504c1d4f5f480
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  890276 , 875161,  891718 
Reviewed-on: https://chromium-review.googlesource.com/c/1258914
Reviewed-by: Stephen McGruer <smcgruer@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596221}
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/fake_server.h
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_connection.cc
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_output.cc
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_output.h
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_output_manager.cc
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_output_manager.h
[modify] https://crrev.com/b5aaa4ea1ffb2e4cc0ce1dee31027c03dfdf5c67/ui/ozone/platform/wayland/wayland_screen_unittest.cc

Labels: -Sheriff-Chromium
Owner: msi...@igalia.com
Status: Assigned (was: Available)
Removing from sheriff queue. Assigning to owners/reviewer of original CL. Feel free to close or repurpose this issue as you need.
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 9

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

commit 42776a4625e1aa02206a460934d4ce240386e2f6
Author: Maksim Sisov <msisov@igalia.com>
Date: Tue Oct 09 14:54:18 2018

Reland [ozone/wayland] Add remove output functionality

This patch relands 14de2307d2980ecb5282767d3bef6bd568ad282b.

The problem was that global output objects were destroyed
before the client had unbound from those outputs, which
resulted in the stack-use-after-return problem.

To address the issue, new global objects are now stored by
the FakeServer and destroyed upon its destruction now. From
now on, the fix makes it sure that the FakeServer follows
the Wayland documentation, which says that global objects
must be kept alive until a client unbounds from them.

* Original CL's description:

> This CL adds a new functionality to the existing WaylandOutputManager
> and WaylandScreen classes. Now, it is possible to remove and
> change primary displays on hot plugs and removes.
>
> To test that functionality, a new test has also been added.
>
> The logic that assigns output ids has also been changed.
> Now, we are using numeric names that a Wayland compositor
> announces during Global and GlobalRemove calls. That is,
> each global object is assigned an unique numeric name, which can
> be freely used as an id.
>
> Bug:  890276 , 875161
> Change-Id: I775bdd40a946c9f0c5dde89eba6e36c4615e1f21
> Reviewed-on: https://chromium-review.googlesource.com/c/1254146
> Commit-Queue: Maksim Sisov <msisov@igalia.com>
> Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#596155}

Bug:  890276 , 875161,  891718 
Change-Id: I2f8aeebcfb67bda75d7d1ba0b0cd331a5eb50c6f
Reviewed-on: https://chromium-review.googlesource.com/c/1268020
Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
Commit-Queue: Maksim Sisov <msisov@igalia.com>
Cr-Commit-Position: refs/heads/master@{#597912}
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/fake_server.h
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_connection.cc
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_output.cc
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_output.h
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_output_manager.cc
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_output_manager.h
[modify] https://crrev.com/42776a4625e1aa02206a460934d4ce240386e2f6/ui/ozone/platform/wayland/wayland_screen_unittest.cc

Status: Fixed (was: Assigned)

Sign in to add a comment