Page MenuHome

Prefetching can corrupt .blend files
Closed, ResolvedPublicBUG

Description

System Information
Operating system: Windows-10-10.0.18362-SP0 64 Bits
Graphics card: Radeon RX 570 Series ATI Technologies Inc. 4.5.13572 Core Profile Context 19.11.2 26.20.13031.15006

Blender Version
Broken: version: 2.90.0 Alpha, branch: master, commit date: 2020-07-04 15:31, hash: rB464aaf27016f

Short description of error.
Prefetching can corrupt .blend files. Following steps may not work on all hardware. This is quite random issue as far as I can tell.

Exact steps for others to reproduce the error

  1. Open file
  2. Add color strip
  3. Change length to 2500
  4. While color strip is prefetching, save file

.blend file is now broken - seq->effectdata is corrupted.

Event Timeline

On further testing many other actions seem to crash the file, like rendering from the first frame. Just before these crashes began, I used the opacity property to have my intro fade in from black, maybe it's related.

Richard Antalik (ISS) changed the task status from Needs Triage to Confirmed.Jul 13 2020, 3:08 PM
Richard Antalik (ISS) triaged this task as High priority.
Richard Antalik (ISS) changed the subtype of this task from "Report" to "Bug".

Thanks for report.

There seems to be issue with prefetching corrupting .blend files. There have been several reports for this, I have not been able to reproduce this issue until now.

I will leave notes for me to look into this:

  • There is thread race with depsgraph: this seems to happen with color strips mainly. To reproduce, add color strip and copy / paste it several times. .
  • Corruption: have array with color strips, save file while prefetching them.

Modifications of strips should stop and re-initialize the pre-fetch job. This is because prefetch is sharing data from the main database, and can access/write to it from a no-main thread.

There are two possible ways to solve the problem:

  • Any operator and RNA property which affects on sequencer is to stop the prefetch job (similar to the material preview)
  • Or, the prefetch is to become fully autonomous (similar to what render pipeline does with DEG_graph_build_for_render_pipeline). It still would need to be re-started after modifications, but at least this could happen as a "listener" of the dependency graph.

@Sergey Sharybin (sergey) Prefetching is stopped when cache is invalidated, which should be every time some strip is changed. Also I am already using DEG_graph_build_for_render_pipeline, but maybe incorrectly?

Bigger issue is actual file corruption. I am not sure what is mechanism of that yet. I will trace it. Prefetch should not touch any data in bmain other than own structs.

Richard Antalik (ISS) renamed this task from Scrolling in VSE crashes blender. to Prefetching can corrupt .blend files.Jul 14 2020, 10:40 PM
Richard Antalik (ISS) updated the task description. (Show Details)

I thought I fixed this already. It seems that files get corrupted because of typo:

diff --git a/source/blender/blenkernel/intern/sequencer.c b/source/blender/blenkernel/intern/sequencer.c
index bde94f366b9..07caf84474b 100644
--- a/source/blender/blenkernel/intern/sequencer.c
+++ b/source/blender/blenkernel/intern/sequencer.c
@@ -5667,7 +5667,7 @@ static Sequence *seq_dupli(const Scene *scene_src,
     struct SeqEffectHandle sh;
     sh = BKE_sequence_get_effect(seq);
     if (sh.copy) {
-      sh.copy(seq, seqn, flag);
+      sh.copy(seqn, seq, flag);
     }

     seqn->strip->stripdata = NULL;

Second issue I described in comment seems to go away as well, so it only looked like thread race.

Prefetching is stopped when cache is invalidated, which should be every time some strip is changed. Also I am already using DEG_graph_build_for_render_pipeline, but maybe incorrectly?

If you're using DEG_graph_build_for_render_pipeline, then the stopping prefetch on cache invalidation seems fine.

I do see some issues in the seqprefetch.c though. Summary goes as:

  • DEG_graph_build_for_render_pipeline is happen from main thread.
  • Initial depsgraph update is to happen from main thread as well.
  • Depsgraph is never to be freed and re-initialized during prefetch. If something what affects sequencer did change, it should re-initialize the prefetch job.
  • Use DEG_evaluate_on_framechange to update depsgraph for the new frame from the prefetch job.

The reason of this is because first evaluation of the depsgraph needs to run copy-on-write operations, which do access original datablocks. So if the copy-on-write happens from a thread, while someone is modifying strip you will run into threading conflicts.

Building depsgraph and evaluating it once from the main thread makes sure that no user interaction happens during copy-on-write is in progress.

I see issue with initializing depsgraph from thread. I will change to do this from main thread.