summary |
shortlog |
log |
commit | commitdiff |
tree
raw |
patch |
inline | side by side (from parent 1:
1bf2b3e)
If something in the UI wanted to update a light on the MIDI device
at the exact same time the operator pressed a button on said device,
we could get a deadlock. The problem was that MIDIDevice::handle_event()
would lock the MIDIDevice and then go to tell MIDIMapper that the
note was coming (which needs a lock on the MIDIMapper), while in the
other thread, MIDIMapper::refresh_lights() would first lock the MIDIMapper
and then call MIDIDevice::update_lights() with the new set of lights
(which nedes a lock on the MIDIDevice). This is a classic lock ordering
issue.
The solution is to simply make MIDIDevice::handle_event() not lock the
MIDIDevice for anything that calls MIDIMapper; it doesn't actually modify
any state in MIDIDevice, except when we hotplug new devices (and that never
calls MIDIMapper).
- lock_guard<recursive_mutex> lock(mu);
switch (event->type) {
case SND_SEQ_EVENT_CONTROLLER: {
receiver->controller_received(event->data.control.param, event->data.control.value);
switch (event->type) {
case SND_SEQ_EVENT_CONTROLLER: {
receiver->controller_received(event->data.control.param, event->data.control.value);
receiver->note_on_received(event->data.note.note);
break;
}
receiver->note_on_received(event->data.note.note);
break;
}
- case SND_SEQ_EVENT_PORT_START:
+ case SND_SEQ_EVENT_PORT_START: {
+ lock_guard<recursive_mutex> lock(mu);
subscribe_to_port_lock_held(seq, event->data.addr);
break;
subscribe_to_port_lock_held(seq, event->data.addr);
break;
case SND_SEQ_EVENT_PORT_EXIT:
printf("MIDI port %d:%d went away.\n", event->data.addr.client, event->data.addr.port);
break;
case SND_SEQ_EVENT_PORT_EXIT:
printf("MIDI port %d:%d went away.\n", event->data.addr.client, event->data.addr.port);
break;
std::atomic<bool> should_quit{false};
int should_quit_fd;
std::atomic<bool> should_quit{false};
int should_quit_fd;
- mutable std::recursive_mutex mu; // Recursive because the MIDI receiver may update_lights() back while we are sending it stuff.
- MIDIReceiver *receiver; // Under <mu>.
+ // Recursive because the MIDI receiver may update_lights() back while we are sending it stuff.
+ // TODO: Do we need this anymore after receiver is not under the lock?
+ mutable std::recursive_mutex mu;
+ MIDIReceiver *receiver; // _Not_ under <mu>; everything on it should be thread-safe.
std::thread midi_thread;
std::map<LightKey, uint8_t> current_light_status; // Keyed by note number. Under <mu>.
std::thread midi_thread;
std::map<LightKey, uint8_t> current_light_status; // Keyed by note number. Under <mu>.