Fix crash in PropagateRemoteMkdir

Fixes: #9170
owncloud · Nov 22, 2021 · a0787eb · a0787eb
1 parent 45d631f
commit a0787eb
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 47 deletions.
diff --git a/changelog/unreleased/9170 b/changelog/unreleased/9170
@@ -0,0 +1,6 @@
+Bugfix: Crash when handling locked files
+
+We fixed a crash that could occur when trying to add a locked folder to the databse.
+
+
+https://github.com/owncloud/client/issues/9170
diff --git a/src/libsync/propagateremotemkdir.cpp b/src/libsync/propagateremotemkdir.cpp
@@ -22,8 +22,11 @@
 #include <QFile>
 #include <QLoggingCategory>
 #include <QtConcurrent>
+
+using namespace std::chrono_literals;
+
 namespace {
-auto UpdateMetaDataRetyTimeOut = std::chrono::seconds(30);
+constexpr auto UpdateMetaDataRetyTimeOut = 30s;
 }
 namespace OCC {
 
@@ -130,13 +133,12 @@ void PropagateRemoteMkdir::success()
 {
     // Never save the etag on first mkdir.
     // Only fully propagated directories should have the etag set.
-    auto itemCopy = *_item;
-    itemCopy._etag.clear();
+    auto itemCopy = SyncFileItemPtr::create(*_item);
+    itemCopy->_etag.clear();
 
     // save the file id already so we can detect rename or remove
     // also convert to a placeholder for the proper behaviour of the file
-    Q_ASSERT(thread() == qApp->thread());
-    const auto result = propagator()->updateMetadata(itemCopy);
+    const auto result = propagator()->updateMetadata(*itemCopy);
     if (!result) {
         done(SyncFileItem::FatalError, tr("Error writing metadata to the database: %1").arg(result.error()));
         return;
@@ -148,53 +150,41 @@ void PropagateRemoteMkdir::success()
         // in case the file lock is active.
         // Retry the conversion for UpdateMetaDataRetyTimeOut
         // TODO: make update updateMetadata async in general
-
-        // QtConcurrent does not support moves
-        // use shared_ptr to manage the Result object
-        using resultType = decltype(result);
-        using resultTypePtr = decltype(std::shared_ptr<resultType>());
-
-        auto *watcher = new QFutureWatcher<resultTypePtr>(this);
-        connect(watcher, &QFutureWatcherBase::finished, this, [watcher, this] {
-            const auto *result = watcher->result().get();
-            if (*result) {
-                switch (result->get()) {
-                case Vfs::ConvertToPlaceholderResult::Ok:
-                    done(SyncFileItem::Success);
-                    return;
-                case Vfs::ConvertToPlaceholderResult::Locked:
-                    done(SyncFileItem::SoftError, tr("Setting file status failed due to file lock"));
-                    return;
-                case Vfs::ConvertToPlaceholderResult::Error:
-                    Q_UNREACHABLE();
-                }
-            }
-            done(SyncFileItem::FatalError, result->error());
-        });
-
-        watcher->setFuture(QtConcurrent::run([p = propagator(), itemCopy] {
-            // Try to update the meta data with a 30s timeout
-            const auto start = std::chrono::steady_clock::now();
-            while ((std::chrono::steady_clock::now() - start) < UpdateMetaDataRetyTimeOut) {
-                QThread::sleep(1);
-                auto result = std::make_shared<resultType>(p->updateMetadata(itemCopy));
-                if (*result) {
-                    if (result->get() == Vfs::ConvertToPlaceholderResult::Locked) {
-                        continue;
-                    } else {
-                        return result;
-                    }
-                } else {
-                    return result;
-                }
-            }
-            return std::make_shared<resultType>(Vfs::ConvertToPlaceholderResult::Locked);
-        }));
+        retryUpdateMetadata(std::chrono::steady_clock::now(), itemCopy);
         return;
     }
 #else
     Q_UNUSED(UpdateMetaDataRetyTimeOut);
 #endif
     done(SyncFileItem::Success);
 }
+
+#ifdef Q_OS_WIN
+void PropagateRemoteMkdir::retryUpdateMetadata(std::chrono::steady_clock::time_point start, SyncFileItemPtr item)
+{
+    // Try to update the meta data with a 30s timeout
+    if ((std::chrono::steady_clock::now() - start) < UpdateMetaDataRetyTimeOut) {
+        auto result = propagator()->updateMetadata(*item);
+        if (result) {
+            switch (*result) {
+            case Vfs::ConvertToPlaceholderResult::Ok:
+                done(SyncFileItem::Success);
+                return;
+            case Vfs::ConvertToPlaceholderResult::Locked:
+                // retry in 1s
+                QTimer::singleShot(1s, this, [start, item, this] {
+                    retryUpdateMetadata(start, item);
+                });
+                return;
+            case Vfs::ConvertToPlaceholderResult::Error:
+                Q_UNREACHABLE();
+            }
+        } else {
+            done(SyncFileItem::FatalError, result.error());
+            return;
+        }
+    }
+    done(SyncFileItem::SoftError, tr("Setting file status failed due to file lock"));
+};
+#endif
 }
diff --git a/src/libsync/propagateremotemkdir.h b/src/libsync/propagateremotemkdir.h
@@ -52,5 +52,11 @@ private slots:
     void slotStartMkcolJob();
     void slotMkcolJobFinished();
     void success();
+
+private:
+#ifdef Q_OS_WIN
+    void retryUpdateMetadata(std::chrono::steady_clock::time_point start, SyncFileItemPtr item);
+
+#endif
 };
 }