Page MenuHome

Fix T76274: Thread race condition on undo with prefetching enabled
ClosedPublic

Authored by Richard Antalik (ISS) on May 6 2020, 7:15 AM.

Diff Detail

Repository
rB Blender
Branch
T76320 (branched from master)
Build Status
Buildable 7903
Build 7903: arc lint + arc unit

Event Timeline

Richard Antalik (ISS) requested review of this revision.May 6 2020, 7:16 AM
This revision is now accepted and ready to land.May 6 2020, 12:01 PM

Confusing thing here is the maniphest task reference. Is it T76274 or T76320?

Confusing thing here is the maniphest task reference. Is it T76274 or T76320?

Since I wasn't able to reproduce T76274 as reported (No .blend file corruption), I will probably merge it to T76320 with assumption, that issue will go away.
I have no idea how any data corruption may happen outside of race condition.

Brecht Van Lommel (brecht) requested changes to this revision.May 6 2020, 11:11 PM

I think this code should be moved into WM_jobs_kill_all, and it should stop prefetching in all scenes, not just the active one.

This revision now requires changes to proceed.May 6 2020, 11:11 PM

I think this code should be moved into WM_jobs_kill_all, and it should stop prefetching in all scenes, not just the active one.

Do you mean by that that I should use WM_jobs API, so that prefetch job is added to wm->jobs?
I could do that, I am not sure why I didn't. I guess I thought that prefetch would be fundamentally inpompatible, but it doesn't really look like it is.

For this fix prefetch doesn't have to be moved to the jobs system, just the call to BKE_sequencer_prefetch_stop() should be moved into WM_jobs_kill_all() so that it is called from all the same places.

Letting prefetch use the jobs system is fine as well, but probably better for master than 2.83.

For this fix prefetch doesn't have to be moved to the jobs system, just the call to BKE_sequencer_prefetch_stop() should be moved into WM_jobs_kill_all() so that it is called from all the same places.

WM_jobs_kill_all() would have to have access to bmain to see all prefetch jobs - they are owned by scene.

I can make BKE_sequencer_prefetch_stop_all() and access bmain global...

I can make BKE_sequencer_prefetch_stop_all() and access bmain global...

I think that's fine.

Or just add main as an argument to WM_jobs_kill_all.

  • Free all prefetch jobs on undo
This revision is now accepted and ready to land.May 11 2020, 8:20 PM