possible race condition with ConfigObjects

Bug #1428500 reported by Daniel Schürmann on 2015-03-05
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mixxx
Critical
Daniel Schürmann
2.0
Undecided
Daniel Schürmann
2.1
Undecided
Daniel Schürmann

Bug Description

ConfigObjects are not thread save, but accessed from different threads.
They are also stored in a non thread save Qlist.

We should add a mutex to guarantee that no bad things can happen.
In addition, we should remove all access from other than the main thread.

Changed in mixxx:
importance: Undecided → Critical
milestone: none → 1.12.0
Changed in mixxx:
assignee: nobody → Daniel Schürmann (daschuer)
RJ Ryan (rryan) wrote :

Yes, this has been the case for years and we've never seen an issue. Adding a mutex should be fine.

> In addition, we should remove all access from other than the main thread.

Why? The idea behind ConfigObject is to easily share the user's preferences. Having to make a proxy CO for every configkey is more likely to introduce out-of-sync errors. We only need proxy COs for communicating config changes to the engine IMO since we shouldn't be locking a mutex there.

Daniel Schürmann (daschuer) wrote :

> Why? The idea ...

Ah yes, just locking a mutex is fine ...

I have added the mutex to https://github.com/mixxxdj/mixxx/pull/501

Together with the move to QHash the lookups a reasonable fast, so that we could use them form everywhere, except where performance is a real issue. (engine Thread)

Changed in mixxx:
status: New → In Progress
RJ Ryan (rryan) on 2016-01-09
Changed in mixxx:
milestone: 2.0.0 → none
Changed in mixxx:
milestone: none → 2.1.0
status: In Progress → Fix Committed
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers