From 8209bd3ecf7d5d9eaa5303928e9ea0b707214b73 Mon Sep 17 00:00:00 2001 From: Kristian Bendiksen Date: Wed, 17 Jun 2026 11:40:48 +0200 Subject: [PATCH] #14230 Fix deadlock when importing wells from OSDU The GUI thread deadlocked on a non-recursive QMutex during OSDU well import. The download-completion slots and DataLoadController::onTaskFinished held a mutex while triggering progress updates, which call QApplication::processEvents(). That re-enters the event loop and dispatches another queued parquet-download-complete event for a different well, whose handler then blocks trying to lock the same mutex. Release the mutex before emitting taskDone in the OSDU well path and well log data loaders, and before calling incrementProgress() in onTaskFinished. The map access that the mutex protects stays inside the locked scope; the captured ProgressInfo outlives the call (kept alive by blockUntilDone) and incrementProgress() is thread-safe via self-marshaling to the GUI thread. Fixes #14230. --- .../WellLog/RimOsduWellLogDataLoader.cpp | 17 +++++++++++------ .../WellPath/RimOsduWellPathDataLoader.cpp | 17 +++++++++++------ .../cafDataLoader/cafDataLoadController.cpp | 15 ++++++++++++--- 3 files changed, 34 insertions(+), 15 deletions(-) diff --git a/ApplicationLibCode/ProjectDataModel/WellLog/RimOsduWellLogDataLoader.cpp b/ApplicationLibCode/ProjectDataModel/WellLog/RimOsduWellLogDataLoader.cpp index fd183e19ac..fdc638dbd8 100644 --- a/ApplicationLibCode/ProjectDataModel/WellLog/RimOsduWellLogDataLoader.cpp +++ b/ApplicationLibCode/ProjectDataModel/WellLog/RimOsduWellLogDataLoader.cpp @@ -87,11 +87,13 @@ bool RimOsduWellLogDataLoader::isRunnable() const //-------------------------------------------------------------------------------------------------- void RimOsduWellLogDataLoader::parquetDownloadComplete( const QByteArray& contents, const QString& url, const QString& id ) { - QMutexLocker lock( &m_mutex ); - - if ( m_wellLogs.find( id ) != m_wellLogs.end() ) + int taskId = -1; { - int taskId = m_taskIds[id]; + QMutexLocker lock( &m_mutex ); + + if ( m_wellLogs.find( id ) == m_wellLogs.end() ) return; + + taskId = m_taskIds[id]; RiaLogging::info( std::format( "Parquet download complete. Id: {} Size: {}", id, contents.size() ) ); @@ -109,7 +111,10 @@ void RimOsduWellLogDataLoader::parquetDownloadComplete( const QByteArray& conten RiaLogging::warning( errorMessage.toStdString() ); } } - - taskDone.send( m_dataType, taskId ); } + + // The lock is released above before emitting taskDone. The signal is delivered synchronously and triggers + // progress updates that call QApplication::processEvents(), which can re-enter this slot for another + // well. Holding the non-recursive m_mutex across send() would then deadlock the GUI thread (#14230). + taskDone.send( m_dataType, taskId ); } diff --git a/ApplicationLibCode/ProjectDataModel/WellPath/RimOsduWellPathDataLoader.cpp b/ApplicationLibCode/ProjectDataModel/WellPath/RimOsduWellPathDataLoader.cpp index 8fe31f0ddb..46d8f2d739 100644 --- a/ApplicationLibCode/ProjectDataModel/WellPath/RimOsduWellPathDataLoader.cpp +++ b/ApplicationLibCode/ProjectDataModel/WellPath/RimOsduWellPathDataLoader.cpp @@ -102,12 +102,14 @@ void RimOsduWellPathDataLoader::cancel() //-------------------------------------------------------------------------------------------------- void RimOsduWellPathDataLoader::parquetDownloadComplete( const QByteArray& contents, const QString& url, const QString& id ) { - QMutexLocker lock( &m_mutex ); - - if ( m_wellPaths.find( id ) != m_wellPaths.end() && m_taskIds.find( id ) != m_taskIds.end() ) + int taskId = -1; { + QMutexLocker lock( &m_mutex ); + + if ( m_wellPaths.find( id ) == m_wellPaths.end() || m_taskIds.find( id ) == m_taskIds.end() ) return; + RiaLogging::info( std::format( "Parquet download complete. Id: {} Size: {}", id, contents.size() ) ); - int taskId = m_taskIds[id]; + taskId = m_taskIds[id]; if ( !contents.isEmpty() ) { @@ -122,7 +124,10 @@ void RimOsduWellPathDataLoader::parquetDownloadComplete( const QByteArray& conte RiaLogging::warning( errorMessage.toStdString() ); } } - - taskDone.send( m_dataType, taskId ); } + + // The lock is released above before emitting taskDone. The signal is delivered synchronously and triggers + // progress updates that call QApplication::processEvents(), which can re-enter this slot for another + // well. Holding the non-recursive m_mutex across send() would then deadlock the GUI thread (#14230). + taskDone.send( m_dataType, taskId ); } diff --git a/Fwk/AppFwk/cafDataLoader/cafDataLoadController.cpp b/Fwk/AppFwk/cafDataLoader/cafDataLoadController.cpp index e2b598fe82..360e236375 100644 --- a/Fwk/AppFwk/cafDataLoader/cafDataLoadController.cpp +++ b/Fwk/AppFwk/cafDataLoader/cafDataLoadController.cpp @@ -141,7 +141,16 @@ void DataLoadController::blockUntilDone( const QString& dataType ) //-------------------------------------------------------------------------------------------------- void DataLoadController::onTaskFinished( const caf::SignalEmitter* emitter, QString dataType, int taskId ) { - QMutexLocker locker( &m_mutex ); - m_pendingTasksByType[dataType]--; - if ( m_progressInfos.find( dataType ) != m_progressInfos.end() ) m_progressInfos[dataType]->incrementProgress(); + ProgressInfo* progressInfo = nullptr; + { + QMutexLocker locker( &m_mutex ); + m_pendingTasksByType[dataType]--; + auto it = m_progressInfos.find( dataType ); + if ( it != m_progressInfos.end() ) progressInfo = it->second; + } + + // Update progress after releasing the lock. incrementProgress() calls QApplication::processEvents(), + // which can re-enter this slot for another finished task. Holding the non-recursive m_mutex across that + // re-entrant call would deadlock the GUI thread (#14230). + if ( progressInfo ) progressInfo->incrementProgress(); }