Lv2Proc: Delay setting worker iface (Fixes #6946) (#6947)

* Lv2Proc: Delay worker iface (Fixes #6946) (#6947)

This delays passing the `LV2_Worker_Interface` to the `Lv2Worker` class,
because prior to the patch, the instance, which provides the interface,
has not been initialized yet, which resulted in a segfault.

* Update src/core/lv2/Lv2Worker.cpp

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>

* setIface -> setInterface

* `if(` -> `if (`

* Update src/core/lv2/Lv2Proc.cpp

Co-authored-by: saker <sakertooth@gmail.com>

* Rework, editorial, from @sakertooth

* Fixup: `interface` is reserved on MSVC

https://stackoverflow.com/a/25234279

* Apply suggestions from code review

Co-authored-by: saker <sakertooth@gmail.com>

* Initialize handle/interface as nullptr

---------

Co-authored-by: Dalton Messmer <33463986+messmerd@users.noreply.github.com>
Co-authored-by: saker <sakertooth@gmail.com>

include/Lv2Worker.h

@@ -47,16 +47,17 @@ class Lv2Worker
 {
 public:
 	// CTOR/DTOR/feature access
-	Lv2Worker(const LV2_Worker_Interface* iface, Semaphore* common_work_lock, bool threaded);
+	Lv2Worker(Semaphore* commonWorkLock, bool threaded);
 	~Lv2Worker();
-	void setHandle(LV2_Handle handle) { m_handle = handle; }
+	void setHandle(LV2_Handle handle);
+	void setInterface(const LV2_Worker_Interface* newInterface);
 	LV2_Worker_Schedule* feature() { return &m_scheduleFeature; }
 
 	// public API
 	void emitResponses();
 	void notifyPluginThatRunFinished()
 	{
-		if(m_iface->end_run) { m_iface->end_run(m_scheduleFeature.handle); }
+		if(m_interface->end_run) { m_interface->end_run(m_scheduleFeature.handle); }
 	}
 
 	// to be called only by static functions
@@ -69,9 +70,9 @@ private:
 	std::size_t bufferSize() const;  //!< size of internal buffers
 
 	// parameters
-	const LV2_Worker_Interface* m_iface;
-	bool m_threaded;
-	LV2_Handle m_handle;
+	const bool m_threaded;
+	const LV2_Worker_Interface* m_interface = nullptr;
+	LV2_Handle m_handle = nullptr;
 	LV2_Worker_Schedule m_scheduleFeature;
 
 	// threading/synchronization

src/core/lv2/Lv2Proc.cpp

@@ -442,11 +442,16 @@ void Lv2Proc::initPlugin()
 
 	if (m_instance)
 	{
-		if(m_worker) {
+		const auto iface = static_cast<const LV2_Worker_Interface*>(
+			lilv_instance_get_extension_data(m_instance, LV2_WORKER__interface));
+		if (iface) {
 			m_worker->setHandle(lilv_instance_get_handle(m_instance));
+			m_worker->setInterface(iface);
 		}
 		for (std::size_t portNum = 0; portNum < m_ports.size(); ++portNum)
+		{
 			connectPort(portNum);
+		}
 		lilv_instance_activate(m_instance);
 	}
 	else
@@ -528,12 +533,10 @@ void Lv2Proc::initPluginSpecificFeatures()
 	// worker (if plugin has worker extension)
 	Lv2Manager* mgr = Engine::getLv2Manager();
 	if (lilv_plugin_has_extension_data(m_plugin, mgr->uri(LV2_WORKER__interface).get())) {
-		const auto iface = static_cast<const LV2_Worker_Interface*>(
-			lilv_instance_get_extension_data(m_instance, LV2_WORKER__interface));
 		bool threaded = !Engine::audioEngine()->renderOnly();
-		m_worker.emplace(iface, &m_workLock, threaded);
+		m_worker.emplace(&m_workLock, threaded);
 		m_features[LV2_WORKER__schedule] = m_worker->feature();
-		// Note: m_worker::setHandle will still need to be called later
+		// note: the worker interface can not be instantiated yet - it requires m_instance. see initPlugin()
 	}
 }
 

src/core/lv2/Lv2Worker.cpp

@@ -60,10 +60,7 @@ std::size_t Lv2Worker::bufferSize() const
 
 
 
-Lv2Worker::Lv2Worker(const LV2_Worker_Interface* iface,
-	Semaphore* common_work_lock,
-	bool threaded) :
-	m_iface(iface),
+Lv2Worker::Lv2Worker(Semaphore* commonWorkLock, bool threaded) :
 	m_threaded(threaded),
 	m_response(bufferSize()),
 	m_requests(bufferSize()),
@@ -71,9 +68,8 @@ Lv2Worker::Lv2Worker(const LV2_Worker_Interface* iface,
 	m_requestsReader(m_requests),
 	m_responsesReader(m_responses),
 	m_sem(0),
-	m_workLock(common_work_lock)
+	m_workLock(commonWorkLock)
 {
-	assert(iface);
 	m_scheduleFeature.handle = static_cast<LV2_Worker_Schedule_Handle>(this);
 	m_scheduleFeature.schedule_work = [](LV2_Worker_Schedule_Handle handle,
 		uint32_t size, const void* data) -> LV2_Worker_Status
@@ -91,6 +87,24 @@ Lv2Worker::Lv2Worker(const LV2_Worker_Interface* iface,
 
 
 
+void Lv2Worker::setHandle(LV2_Handle handle)
+{
+	assert(handle);
+	m_handle = handle;
+}
+
+
+
+
+void Lv2Worker::setInterface(const LV2_Worker_Interface* newInterface)
+{
+	assert(newInterface);
+	m_interface = newInterface;
+}
+
+
+
+
 Lv2Worker::~Lv2Worker()
 {
 	m_exit = true;
@@ -120,7 +134,9 @@ LV2_Worker_Status Lv2Worker::respond(uint32_t size, const void* data)
 	}
 	else
 	{
-		m_iface->work_response(m_handle, size, data);
+		assert(m_handle);
+		assert(m_interface);
+		m_interface->work_response(m_handle, size, data);
 	}
 	return LV2_WORKER_SUCCESS;
 }
@@ -136,6 +152,7 @@ void Lv2Worker::workerFunc()
 	while (true) {
 		m_sem.wait();
 		if (m_exit) { break; }
+
 		const std::size_t readSpace = m_requestsReader.read_space();
 		if (readSpace <= sizeof(size)) { continue; } // (should not happen)
 
@@ -144,8 +161,10 @@ void Lv2Worker::workerFunc()
 		if(size > buf.size()) { buf.resize(size); }
 		if(size) { m_requestsReader.read(size).copy(buf.data(), size); }
 
+		assert(m_handle);
+		assert(m_interface);
 		m_workLock->wait();
-		m_iface->work(m_handle, staticWorkerRespond, this, size, buf.data());
+		m_interface->work(m_handle, staticWorkerRespond, this, size, buf.data());
 		m_workLock->post();
 	}
 }
@@ -172,9 +191,11 @@ LV2_Worker_Status Lv2Worker::scheduleWork(uint32_t size, const void *data)
 	}
 	else
 	{
+		assert(m_handle);
+		assert(m_interface);
 		// Execute work immediately in this thread
 		m_workLock->wait();
-		m_iface->work(m_handle, staticWorkerRespond, this, size, data);
+		m_interface->work(m_handle, staticWorkerRespond, this, size, data);
 		m_workLock->post();
 	}
 
@@ -189,10 +210,13 @@ void Lv2Worker::emitResponses()
 {
 	std::size_t read_space = m_responsesReader.read_space();
 	uint32_t size;
-	while (read_space > sizeof(size)) {
+	while (read_space > sizeof(size))
+	{
+		assert(m_handle);
+		assert(m_interface);
 		m_responsesReader.read(sizeof(size)).copy((char*)&size, sizeof(size));
 		if(size) { m_responsesReader.read(size).copy(m_response.data(), size); }
-		m_iface->work_response(m_handle, size, m_response.data());
+		m_interface->work_response(m_handle, size, m_response.data());
 		read_space -= sizeof(size) + size;
 	}
 }