Fix memory error in remove_audio_processor()

We would close the socket, but the `receive_multi()` call would finish
after the object had already been deallocated using `erase()`. Somehow
this never caused any issues though.
This commit is contained in:
Robbert van der Helm
2021-04-07 17:08:01 +02:00
parent 86b9ad5c8a
commit 3ae4bf56cd
2 changed files with 26 additions and 0 deletions
+2
View File
@@ -14,6 +14,8 @@ Versioning](https://semver.org/spec/v2.0.0.html).
shutting down. With Wine 6.5 terminating a Wine process no longer terminates shutting down. With Wine 6.5 terminating a Wine process no longer terminates
its threads, which would cause yabridge's plugin and host components to wait its threads, which would cause yabridge's plugin and host components to wait
for each other to shut down. for each other to shut down.
- Fixed a multithreading related memory error in the VST3 audio processor socket
management.
### yabridgectl ### yabridgectl
+24
View File
@@ -526,6 +526,12 @@ class AdHocSocketHandler {
socket.shutdown( socket.shutdown(
boost::asio::local::stream_protocol::socket::shutdown_both, err); boost::asio::local::stream_protocol::socket::shutdown_both, err);
socket.close(); socket.close();
while (currently_listening) {
// If another thread is currently calling `receive_multi()`, we'll
// spinlock until that function has exited. We would otherwise get a
// use-after-free when this object is destroyed from another thread.
}
} }
protected: protected:
@@ -621,6 +627,12 @@ class AdHocSocketHandler {
void receive_multi(std::optional<std::reference_wrapper<Logger>> logger, void receive_multi(std::optional<std::reference_wrapper<Logger>> logger,
F primary_callback, F primary_callback,
G secondary_callback) { G secondary_callback) {
// We use this flag to have the `close()` function wait for the this
// function to exit, to prevent use-after-frees when destroying this
// object from another thread.
assert(!currently_listening);
currently_listening = true;
// As described above we'll handle incoming requests for `socket` on // As described above we'll handle incoming requests for `socket` on
// this thread. We'll also listen for incoming connections on `endpoint` // this thread. We'll also listen for incoming connections on `endpoint`
// on another thread. For any incoming connection we'll spawn a new // on another thread. For any incoming connection we'll spawn a new
@@ -686,6 +698,8 @@ class AdHocSocketHandler {
std::lock_guard lock(active_secondary_requests_mutex); std::lock_guard lock(active_secondary_requests_mutex);
secondary_context.stop(); secondary_context.stop();
acceptor.reset(); acceptor.reset();
currently_listening = false;
} }
/** /**
@@ -766,6 +780,16 @@ class AdHocSocketHandler {
*/ */
std::optional<boost::asio::local::stream_protocol::acceptor> acceptor; std::optional<boost::asio::local::stream_protocol::acceptor> acceptor;
/**
* After the socket gets closed, we do some cleanup at the end of
* `receive_multi()`. To prevent use-after-frees, we should wait for this
* function to exit when `close()`-ing a socket that's currently being
* listened on. Since after closing the socket the thread should terminate
* near instantly, we'll just do a spinlock here instead of using condition
* variables.
*/
std::atomic_bool currently_listening = false;
/** /**
* A mutex that locks the primary `socket`. If this is locked, then any new * A mutex that locks the primary `socket`. If this is locked, then any new
* events will be sent over a new socket instead. * events will be sent over a new socket instead.