@afakhry - I'm not able to reproduce this. Please let me know if you can?

I already fixed this before.

Seems to reproduce still, on dev channel:
https://listnr.corp.google.com/report/74911711501

I could repro. Will look into it.

The root cause of this problem is that the ID of the displays are not stable. For example, I have two external monitors: DELL and HP. Their IDs when both are connected via USB-C to an Eve device are:

DELL: 4693050335952642
HP: 9834342374781185

When you disconnect the HP display, the ID of the DELL display changes to become:

DELL: 4693050335952641

This means that the output index changes, which is used here [1] to generate the display ID. Therefore HardwareDisplayControllerInfo::index() changes (that's the different '2' and '1' in the above two examples). That index is used here [2].


Assigning to dcastagna@ as a DRM owner. Please take a look. Thank you!


[1]: https://cs.chromium.org/chromium/src/ui/display/util/display_util.cc?l=46
[2]: https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/common/drm_util.cc?type=cs&q=CreateDisplaySnapshot&l=342

I don't think displayid was ever supposed to be stable.

How is the display manager/configurator using it?

The display ID is not supposed to be stable, but the port index should be. i.e. If the same display remains connected to the same port, then its ID shouldn't change.

I first assumed that the connector index is something we get from the kernel, but it turned out to be something that we at Chrome calculate it here: https://cs.chromium.org/chromium/src/ui/ozone/platform/drm/common/drm_util.cc?q=GetAvailableDisplayControllerInfos&sq=package:chromium&l=308

The root cause is a memory corruption bug. It used to be that |available_connectors| owned the (connected) connectors while |connectors| just referred to them only, and hence if a DRM connector was *disconnected*, it gets destructed immediately and |connectors| referring to an invalid memory. drmMalloc was allocating the next connector in the same block that was just freed.

This also resulted in |connectors| having entries with equal pointer values resulting in std::find finding the wrong index.

Thanks to Oshima-san for suggesting where to look!

CL is up for review: https://chromium-review.googlesource.com/c/chromium/src/+/838845

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

commit e65b91e0f2203c383a2fbdd4a29f2a323f1c7021
Author: Ahmed Fakhry <afakhry@chromium.org>
Date: Thu Dec 21 23:32:44 2017

Fix a memory corruption bug resulting in wrong claculation of connector index

It used to be that |available_connectors| owned the connectors, and
hence if a DRM connector was disconnected, it gets destructed immediately
resulting in drmMalloc allocating the next connector in the same block.

This resulted in |connectors| having entries with equal pointer values
resulting in std::find finding the wrong index.

BUG= 717371 

Change-Id: I258406ed2261eb9a8cf43143ed30b088cdef2b8e
Reviewed-on: https://chromium-review.googlesource.com/838845
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525851}
[modify] https://crrev.com/e65b91e0f2203c383a2fbdd4a29f2a323f1c7021/ui/ozone/platform/drm/common/drm_util.cc

Since this is serious, I want to merge to M-64.

This wasn't a regression for M64, however.  It was reported in M58, and has been open for some time.  Or was this a new artifact introduce since then?

This bug requires manual review: M64 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), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

See #17

It's not a regression, but it's memory corruption bug, and I think we should merge the fix in M-64 before releasing it.

Approving merge to M64 Chrome OS since this will improve stability.

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

commit 00aa63f3fd54b037570462708b5f869567102f85
Author: Ahmed Fakhry <afakhry@chromium.org>
Date: Thu Dec 28 19:07:03 2017

[Merge to M64] Fix a memory corruption bug resulting in wrong claculation of connector index

It used to be that |available_connectors| owned the connectors, and
hence if a DRM connector was disconnected, it gets destructed immediately
resulting in drmMalloc allocating the next connector in the same block.

This resulted in |connectors| having entries with equal pointer values
resulting in std::find finding the wrong index.

TBR=dcastagna@chromium.org
BUG= 717371 

(cherry picked from commit e65b91e0f2203c383a2fbdd4a29f2a323f1c7021)

Change-Id: I258406ed2261eb9a8cf43143ed30b088cdef2b8e
Reviewed-on: https://chromium-review.googlesource.com/838845
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#525851}
Reviewed-on: https://chromium-review.googlesource.com/845999
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/branch-heads/3282@{#368}
Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840}
[modify] https://crrev.com/00aa63f3fd54b037570462708b5f869567102f85/ui/ozone/platform/drm/common/drm_util.cc