Skip to content

daemon/controllers: Reintroduce a mutex around ddca_open_display2()

Jakob Petsovits requested to merge work/jpetso/ddcutil-display-mutex into master

ddcutil keeps some global state, and in particular we've seen a crash in ddc_close_display() related to its open_displays hashmap.

One likely culprit for such a crash is that brightness workers run on separate threads, one per DDCutilDisplay object, so they could get executed simultaneously when more than one external monitor is involved.

A less likely, but still possible culprit is when a DDCutilDisplay constructor opens a display handle at the same time that a brightness worker sets the brightness of a different display on a different thread.

We could look into combining all brightness setter calls into a single BrightnessWorker that gets shared across DDCutilDisplay objects, but this would introduce extra complexity and still wouldn't address the constructor vs. brightness setter issue.

So this commit reintroduces a mutex to isolate all of these usages from each other. We had one in 5.x, which was removed in 6.0 at my (in hindsight incorrect) request: !312 (comment 865976), !312 (comment 867942)

BUG: 489169

CC @littlesweet

Test plan

  • Move the brightness slider around like a maniac.
  • Enable and disable monitors in the output configuration, or by using the monitor's own hardware switch.
  • Try both of these things at the same time for stressing the code even harder.

Sometimes this should crash without this patch but not with the patch applied.

Edited by Jakob Petsovits

Merge request reports