Skip to content

Commit

Permalink
Fix a deadlock in remote COPC handling (3D views)
Browse files Browse the repository at this point in the history
This gets often triggered from 3D views when the hierachy gets fetched,
because a QEventLoop gets started in the main thread while waiting
for network request to be finished. The event loop was running with
hierarchy mutex locked, which was causing deadlocks when other events
ended up trying to access the hierarchy. This fix unlocks the mutex while
the hierarchy network request is ongoing (the same approach as remote
EPT implementation uses, which seems to work fine).

Looking at the copc/ept hierarchy handling, it is not great - both
formats are conceptually the same, yet the hierarchy implementations
are different, and there are also non-trivial differences between
local/remote datasets. All of this could get some code cleanups
(also unifying local/remote access), but at this time let's just try
to fix the worst bit, and hope something else does not break...
  • Loading branch information
wonder-sk committed Feb 16, 2024
1 parent ef1d7a1 commit a05732d
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/core/pointcloud/qgscopcpointcloudindex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,6 @@ qint64 QgsCopcPointCloudIndex::pointCount() const

bool QgsCopcPointCloudIndex::loadHierarchy()
{
QMutexLocker locker( &mHierarchyMutex );
fetchHierarchyPage( mCopcInfoVlr.root_hier_offset, mCopcInfoVlr.root_hier_size );
return true;
}
Expand Down Expand Up @@ -316,6 +315,8 @@ void QgsCopcPointCloudIndex::fetchHierarchyPage( uint64_t offset, uint64_t byteS
int32_t pointCount;
};

QMutexLocker locker( &mHierarchyMutex );

for ( uint64_t i = 0; i < byteSize; i += sizeof( CopcEntry ) )
{
CopcEntry *entry = reinterpret_cast<CopcEntry *>( data.get() + i );
Expand Down
3 changes: 3 additions & 0 deletions src/core/pointcloud/qgsremotecopcpointcloudindex.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ bool QgsRemoteCopcPointCloudIndex::fetchNodeHierarchy( const IndexedPointCloudNo
if ( nodesCount < 0 )
{
auto hierarchyNodePos = mHierarchyNodePos.constFind( n );
locker.unlock();
fetchHierarchyPage( hierarchyNodePos->first, hierarchyNodePos->second );
}
}
Expand Down Expand Up @@ -227,6 +228,8 @@ void QgsRemoteCopcPointCloudIndex::fetchHierarchyPage( uint64_t offset, uint64_t
int32_t pointCount;
};

QMutexLocker locker( &mHierarchyMutex );

for ( uint64_t i = 0; i < byteSize; i += sizeof( CopcEntry ) )
{
CopcEntry *entry = reinterpret_cast<CopcEntry *>( data.data() + i );
Expand Down

0 comments on commit a05732d

Please sign in to comment.