Prevent rare race conditions with get/setParameter

I've never seen it happen, but in theory they could be called
simultaneously.
This commit is contained in:
Robbert van der Helm
2020-04-19 16:14:22 +02:00
parent 9f3ed85208
commit b13d7d554d
3 changed files with 32 additions and 16 deletions
+3 -3
View File
@@ -102,7 +102,7 @@ class DefaultDataConverter {
* *
* @param socket The socket to write over, should be the same socket the other * @param socket The socket to write over, should be the same socket the other
* endpoint is using to call `passthrough_event()`. * endpoint is using to call `passthrough_event()`.
* @param write_semaphore A mutex to ensure that only one thread can write to * @param write_mutex A mutex to ensure that only one thread can write to
* the socket at once. Needed because VST hosts and plugins can and sometimes * the socket at once. Needed because VST hosts and plugins can and sometimes
* will call the `dispatch()` or `audioMaster()` functions from multiple * will call the `dispatch()` or `audioMaster()` functions from multiple
* threads at once. * threads at once.
@@ -120,7 +120,7 @@ class DefaultDataConverter {
*/ */
template <typename D> template <typename D>
intptr_t send_event(boost::asio::local::stream_protocol::socket& socket, intptr_t send_event(boost::asio::local::stream_protocol::socket& socket,
std::mutex& write_semaphore, std::mutex& write_mutex,
D& data_converter, D& data_converter,
std::optional<std::pair<Logger&, bool>> logging, std::optional<std::pair<Logger&, bool>> logging,
int opcode, int opcode,
@@ -153,7 +153,7 @@ intptr_t send_event(boost::asio::local::stream_protocol::socket& socket,
// multiple threads. // multiple threads.
EventResult response; EventResult response;
{ {
std::lock_guard lock(write_semaphore); std::lock_guard lock(write_mutex);
write_object(socket, event); write_object(socket, event);
response = read_object<EventResult>(socket); response = read_object<EventResult>(socket);
} }
+21 -8
View File
@@ -290,7 +290,7 @@ intptr_t HostBridge::dispatch(AEffect* /*plugin*/,
intptr_t return_value = 1; intptr_t return_value = 1;
try { try {
return_value = return_value =
send_event(host_vst_dispatch, dispatch_semaphore, converter, send_event(host_vst_dispatch, dispatch_mutex, converter,
std::pair<Logger&, bool>(logger, true), opcode, std::pair<Logger&, bool>(logger, true), opcode,
index, value, data, option); index, value, data, option);
} catch (const boost::system::system_error& a) { } catch (const boost::system::system_error& a) {
@@ -330,14 +330,14 @@ intptr_t HostBridge::dispatch(AEffect* /*plugin*/,
// stop receiving midi data when they have an open dropdowns or // stop receiving midi data when they have an open dropdowns or
// message box. // message box.
return send_event(host_vst_dispatch_midi_events, return send_event(host_vst_dispatch_midi_events,
dispatch_midi_events_semaphore, converter, dispatch_midi_events_mutex, converter,
std::pair<Logger&, bool>(logger, true), opcode, std::pair<Logger&, bool>(logger, true), opcode,
index, value, data, option); index, value, data, option);
break; break;
} }
// TODO: Maybe reuse buffers here when dealing with chunk data // TODO: Maybe reuse buffers here when dealing with chunk data
return send_event(host_vst_dispatch, dispatch_semaphore, converter, return send_event(host_vst_dispatch, dispatch_mutex, converter,
std::pair<Logger&, bool>(logger, true), opcode, index, std::pair<Logger&, bool>(logger, true), opcode, index,
value, data, option); value, data, option);
} }
@@ -374,9 +374,16 @@ float HostBridge::get_parameter(AEffect* /*plugin*/, int index) {
logger.log_get_parameter(index); logger.log_get_parameter(index);
const Parameter request{index, std::nullopt}; const Parameter request{index, std::nullopt};
write_object(host_vst_parameters, request); ParameterResult response;
// Prevent race conditions from `getParameter()` and `setParameter()` being
// called at the same time since they share the same socket
{
std::lock_guard lock(parameters_mutex);
write_object(host_vst_parameters, request);
response = read_object<ParameterResult>(host_vst_parameters);
}
const auto response = read_object<ParameterResult>(host_vst_parameters);
logger.log_get_parameter_response(response.value.value()); logger.log_get_parameter_response(response.value.value());
return response.value.value(); return response.value.value();
@@ -386,12 +393,18 @@ void HostBridge::set_parameter(AEffect* /*plugin*/, int index, float value) {
logger.log_set_parameter(index, value); logger.log_set_parameter(index, value);
const Parameter request{index, value}; const Parameter request{index, value};
write_object(host_vst_parameters, request); ParameterResult response;
{
std::lock_guard lock(parameters_mutex);
write_object(host_vst_parameters, request);
response = read_object<ParameterResult>(host_vst_parameters);
}
// This should not contain any values and just serve as an acknowledgement
const auto response = read_object<ParameterResult>(host_vst_parameters);
logger.log_set_parameter_response(); logger.log_set_parameter_response();
// This should not contain any values and just serve as an acknowledgement
assert(!response.value.has_value()); assert(!response.value.has_value());
} }
+8 -5
View File
@@ -150,9 +150,6 @@ class HostBridge {
/** /**
* Used for both `getParameter` and `setParameter` since they mostly * Used for both `getParameter` and `setParameter` since they mostly
* overlap. * overlap.
*
* TODO: Verify that these 100% won't be called simultanously since that
* would cause a race condition.
*/ */
boost::asio::local::stream_protocol::socket host_vst_parameters; boost::asio::local::stream_protocol::socket host_vst_parameters;
boost::asio::local::stream_protocol::socket host_vst_process_replacing; boost::asio::local::stream_protocol::socket host_vst_process_replacing;
@@ -174,8 +171,14 @@ class HostBridge {
* being called by two threads at once. See `send_event()` for more * being called by two threads at once. See `send_event()` for more
* information. * information.
*/ */
std::mutex dispatch_semaphore; std::mutex dispatch_mutex;
std::mutex dispatch_midi_events_semaphore; std::mutex dispatch_midi_events_mutex;
/**
* A similar semaphore as the `dispatch_*` semaphores in the rare case that
* `getParameter()` and `setParameter()` are being called at the same time
* since they use the same socket.
*/
std::mutex parameters_mutex;
/** /**
* The callback function passed by the host to the VST plugin instance. * The callback function passed by the host to the VST plugin instance.