possible race condition with ConfigObjects
Bug #1428500 reported by
Daniel Schürmann
This bug affects 1 person
| Affects | Status | Importance | Assigned to | Milestone | |
|---|---|---|---|---|---|
| Mixxx |
Fix Released
|
Critical
|
Daniel Schürmann | ||
| 2.0 |
Won't Fix
|
Undecided
|
Daniel Schürmann | ||
| 2.1 |
Fix Released
|
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) |
Revision history for this message
| RJ Skerry-Ryan (rryan) wrote | #1 |
Revision history for this message
| Daniel Schürmann (daschuer) wrote | #2 |
| Changed in mixxx: | |
| status: | New → In Progress |
| Changed in mixxx: | |
| milestone: | 2.0.0 → none |
| Changed in mixxx: | |
| milestone: | none → 2.1.0 |
| status: | In Progress → Fix Committed |
| Changed in mixxx: | |
| status: | Fix Committed → Fix Released |
Revision history for this message
| Swiftb0y (swiftb0y) wrote | #3 |
| lock status: | Metadata changes locked and limited to project staff |
To post a comment you must log in.
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.