From ea78558a3169c3a6e2e66b38f7e1ecf261c2b50b Mon Sep 17 00:00:00 2001 From: Mike Solar Date: Mon, 13 Jul 2026 13:43:20 +0800 Subject: [PATCH] render: reference-count graph snapshots to prevent premature deletion Proxy generation modifies the project, causing graph snapshots to be rewritten. The old implementation deleted the previous snapshot immediately, even though queued render jobs still held its path. The worker then failed to load the deleted file. Add per-path reference counting: - Increment when a job is created with a snapshot path. - Decrement when the job is processed, cancelled or removed. - Only delete a snapshot when its reference count reaches zero and it is no longer the cached snapshot for the project. Also keep snapshots in the system temp directory as before, since the location itself was not the bug. --- app/render/renderworkerpool.cpp | 106 ++++++++++++++++++++++++-------- app/render/renderworkerpool.h | 9 +++ 2 files changed, 89 insertions(+), 26 deletions(-) diff --git a/app/render/renderworkerpool.cpp b/app/render/renderworkerpool.cpp index 831586609..5e99dbf29 100644 --- a/app/render/renderworkerpool.cpp +++ b/app/render/renderworkerpool.cpp @@ -28,7 +28,6 @@ #include #include #include -#include #include #include #include @@ -461,7 +460,7 @@ bool RenderWorkerPool::RemoveTicket(RenderTicketPtr ticket) } if (!queued_graph_path.isEmpty()) { - CleanupGraphFile(queued_graph_path); + ReleaseGraphPathRef(queued_graph_path); return true; } @@ -470,8 +469,6 @@ bool RenderWorkerPool::RemoveTicket(RenderTicketPtr ticket) void RenderWorkerPool::Shutdown() { - QVector graph_paths_to_clean; - { QMutexLocker locker(&mutex_); stopping_ = true; @@ -479,6 +476,7 @@ void RenderWorkerPool::Shutdown() if (job.ticket) { job.ticket->Cancel(); } + ReleaseGraphPathRefLocked(job.graph_path); } queue_.clear(); for (ActiveJob &active : active_jobs_) { @@ -488,16 +486,12 @@ void RenderWorkerPool::Shutdown() } } for (auto it = graph_cache_.begin(); it != graph_cache_.end(); ++it) { - graph_paths_to_clean.append(it->path); + SetGraphPathCachedLocked(it->path, false); } graph_cache_.clear(); wait_.wakeAll(); } - for (const QString &path : graph_paths_to_clean) { - CleanupGraphFile(path); - } - if (isRunning()) { wait(); } @@ -553,6 +547,7 @@ void RenderWorkerPool::WorkerLoop( mutex_.unlock(); ProcessJob(job, worker_index, local_pool); + ReleaseGraphPathRef(job.graph_path); } } @@ -586,13 +581,14 @@ bool RenderWorkerPool::PrepareJob(RenderTicketPtr ticket, auto it = graph_cache_.find(project_uuid); if (it != graph_cache_.end() && !project->is_modified()) { graph_path = it->path; - //qDebug() << "RenderWorkerPool::PrepareJob: using cached graph snapshot" - // << graph_path; + AddGraphPathRefLocked(graph_path); + qDebug() << "RenderWorkerPool::PrepareJob: using cached graph snapshot" + << graph_path; } else { if (it != graph_cache_.end()) { qDebug() << "RenderWorkerPool::PrepareJob: graph stale, rewriting" << project->is_modified(); - CleanupGraphFile(it->path); + SetGraphPathCachedLocked(it->path, false); graph_cache_.erase(it); } locker.unlock(); @@ -608,6 +604,8 @@ bool RenderWorkerPool::PrepareJob(RenderTicketPtr ticket, } locker.relock(); graph_cache_.insert(project_uuid, {graph_path}); + SetGraphPathCachedLocked(graph_path, true); + AddGraphPathRefLocked(graph_path); } } @@ -622,18 +620,10 @@ bool RenderWorkerPool::PrepareJob(RenderTicketPtr ticket, bool RenderWorkerPool::WriteGraphSnapshot(Project *project, QString *path) { - // Write the snapshot into the application's persistent data directory so - // that the worker process can always reach it, regardless of sandbox - // restrictions or working directory. System temp directories are not - // reliably shared between the editor and its child worker on macOS. - const QString base_dir = QStandardPaths::writableLocation( - QStandardPaths::AppLocalDataLocation); - const QString graph_dir = QDir(base_dir).filePath(QStringLiteral("render-graphs")); - if (!QDir(graph_dir).mkpath(QStringLiteral("."))) { - qWarning() << "RenderWorkerPool failed to create graph snapshot directory" - << graph_dir; - return false; - } + // Keep snapshots in the system temp directory. The previous bug was not the + // temp location itself, but stale snapshots being deleted while queued jobs + // still referenced them. + const QString graph_dir = QDir::tempPath(); QTemporaryFile file(QDir(graph_dir).filePath(QStringLiteral("oak-render-graph-XXXXXX.ove"))); file.setAutoRemove(false); @@ -650,11 +640,14 @@ bool RenderWorkerPool::WriteGraphSnapshot(Project *project, QString *path) if (result.code() != ProjectSerializer::kSuccess || writer.hasError()) { qWarning() << "RenderWorkerPool failed to serialize graph snapshot" - << result.GetDetails(); + << result.GetDetails(); QFile::remove(file.fileName()); return false; } + qDebug() << "RenderWorkerPool wrote graph snapshot" << file.fileName() + << "size" << QFileInfo(file.fileName()).size(); + *path = file.fileName(); return true; } @@ -1204,9 +1197,11 @@ void RenderWorkerPool::ClearGraphCache() { QMutexLocker locker(&mutex_); for (auto it = graph_cache_.begin(); it != graph_cache_.end(); ++it) { - CleanupGraphFile(it->path); + SetGraphPathCachedLocked(it->path, false); } graph_cache_.clear(); + graph_path_ref_count_.clear(); + cached_graph_paths_.clear(); } void RenderWorkerPool::FinishWithFrame(RenderTicketPtr ticket, @@ -1238,8 +1233,67 @@ void RenderWorkerPool::FinishWithFrame(RenderTicketPtr ticket, void RenderWorkerPool::CleanupGraphFile(const QString &path) { if (!path.isEmpty()) { + qDebug() << "RenderWorkerPool cleaning up graph file" << path; QFile::remove(path); } } +void RenderWorkerPool::AddGraphPathRef(const QString &path) +{ + QMutexLocker locker(&mutex_); + AddGraphPathRefLocked(path); +} + +void RenderWorkerPool::AddGraphPathRefLocked(const QString &path) +{ + if (path.isEmpty()) { + return; + } + ++graph_path_ref_count_[path]; +} + +void RenderWorkerPool::ReleaseGraphPathRef(const QString &path) +{ + QMutexLocker locker(&mutex_); + ReleaseGraphPathRefLocked(path); +} + +void RenderWorkerPool::ReleaseGraphPathRefLocked(const QString &path) +{ + if (path.isEmpty()) { + return; + } + auto it = graph_path_ref_count_.find(path); + if (it == graph_path_ref_count_.end()) { + return; + } + if (--(*it) <= 0) { + graph_path_ref_count_.erase(it); + if (!cached_graph_paths_.contains(path)) { + CleanupGraphFile(path); + } + } +} + +void RenderWorkerPool::SetGraphPathCached(const QString &path, bool cached) +{ + QMutexLocker locker(&mutex_); + SetGraphPathCachedLocked(path, cached); +} + +void RenderWorkerPool::SetGraphPathCachedLocked(const QString &path, bool cached) +{ + if (path.isEmpty()) { + return; + } + if (cached) { + cached_graph_paths_.insert(path); + } else { + cached_graph_paths_.remove(path); + if (!graph_path_ref_count_.contains(path)) { + CleanupGraphFile(path); + } + } +} + } // namespace olive diff --git a/app/render/renderworkerpool.h b/app/render/renderworkerpool.h index 929a52151..e19496dba 100644 --- a/app/render/renderworkerpool.h +++ b/app/render/renderworkerpool.h @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -128,6 +129,12 @@ private: void FinishWithFrame(RenderTicketPtr ticket, const ipc::FrameSlotPool &pool, uint32_t slot); void CleanupGraphFile(const QString &path); + void AddGraphPathRef(const QString &path); + void AddGraphPathRefLocked(const QString &path); + void ReleaseGraphPathRef(const QString &path); + void ReleaseGraphPathRefLocked(const QString &path); + void SetGraphPathCached(const QString &path, bool cached); + void SetGraphPathCachedLocked(const QString &path, bool cached); void CancelActiveProcess(qint64 process_id); void SetActiveWorker(int worker_index, RenderTicketPtr ticket, QProcess *worker, qint64 ticket_id); @@ -151,6 +158,8 @@ private: bool stopping_ = false; QVector active_jobs_; QHash graph_cache_; + QHash graph_path_ref_count_; + QSet cached_graph_paths_; static constexpr uint32_t kOutputSlots = 2; static constexpr int kMaxAttempts = 2;