wmTimer, wmJob and WM_report issue - can lead to crash #105369

New Issue

Bastien Montagne · 2023-03-02T11:32:51+01:00

Bastien Montagne commented

2023-03-02 11:32:51 +01:00

The Problem

Report #105160 unveils a critical issue in wmTimer handling.

Essentially, in several places of WM code timers are looped over in that way:

/* Mutable in case the timer gets removed. */
LISTBASE_FOREACH_MUTABLE (wmTimer *, wt, &wm->timers) {
  ...
}

This is actually not working as expected, because during the processing of a wmTimer, other wmTimers may be added or removed and freed, The latter will lead to a crash if it is the next timer in the list (LISTBASE_FOREACH_MUTABLE only protects against removal/freeing of the current item).

The typical identified case if a job timer updating or ending its job, which job callbacks would call WM_report, which ends up removing/freeing a timer and adding a new one for the report 'popup' in the UI.

Proposed Solutions

This issue has potential solutions at several levels.

1. Simple Fix the Current Issue

The issue with WM_report modifying timers could be addressed by having a flag in the WindowManager marking timers as being processed, in which case WM_report (or more precisely wm_add_reports) should not call WM_report_banner_show.

WM_report_banner_show call could then be added at the end of the loops over wmTimer.

2. More General Fix

Making wmTimer loop safe in general could be achieved by deferring their deletion (tagging them to be deleted, and actually deleting them in a second, safe loop).

### The Problem Report #105160 unveils a critical issue in wmTimer handling. Essentially, in several places of WM code timers are looped over in that way: ``` /* Mutable in case the timer gets removed. */ LISTBASE_FOREACH_MUTABLE (wmTimer *, wt, &wm->timers) { ... } ``` This is actually not working as expected, because during the processing of a wmTimer, other wmTimers may be added or removed and freed, The latter will lead to a crash if it is the next timer in the list (`LISTBASE_FOREACH_MUTABLE` only protects against removal/freeing of the current item). The typical identified case if a job timer updating or ending its job, which job callbacks would call `WM_report`, which ends up removing/freeing a timer and adding a new one for the report 'popup' in the UI. ### Proposed Solutions This issue has potential solutions at several levels. #### 1. Simple Fix the Current Issue The issue with `WM_report` modifying timers could be addressed by having a flag in the WindowManager marking timers as being processed, in which case `WM_report` (or more precisely `wm_add_reports`) should not call `WM_report_banner_show`. `WM_report_banner_show` call could then be added at the end of the loops over wmTimer. #### 2. More General Fix Making wmTimer loop safe in general could be achieved by deferring their deletion (tagging them to be deleted, and actually deleting them in a second, safe loop).

Bastien Montagne added the

labels 2023-03-02 11:32:51 +01:00

Bastien Montagne added this to the 3.5 milestone 2023-03-02 11:33:04 +01:00

Bastien Montagne commented

2023-03-02 11:34:17 +01:00

@ideasman42 @brecht, could use your feedback on this one. Think it's a fairly bad issue, and would rather fix it for 3.5 (and backport to previous LTS)?

Brecht Van Lommel commented

2023-03-02 15:45:43 +01:00

I'd go with deferred deletion. I couldn't think of another solution that is watertight.

Bastien Montagne referenced this issue

2023-03-02 18:02:22 +01:00

WM: Fix invalid memory access in wmTimer handling code. #105380

Bastien Montagne referenced this issue from a commit

2023-03-03 15:24:37 +01:00

WM: Fix invalid memory access in wmTimer handling code.

Bastien Montagne commented

2023-03-03 15:34:14 +01:00

Committed as d66672e17a.

Committed as d66672e17a.

Bastien Montagne closed this issue

2023-03-03 15:34:18 +01:00

Blender Bot added the

Status

Archived

label 2023-03-03 15:34:28 +01:00

Bastien Montagne referenced this issue

2023-03-03 16:22:18 +01:00

WM: Fix invalid memory access in wmTimer handling code. 2.93 version #105406

Philipp Oeser referenced this issue from a commit

2023-03-16 12:38:36 +01:00

WM: Fix invalid memory access in wmTimer handling code. 2.93 version

Philipp Oeser referenced this issue from a commit

2023-03-16 15:19:18 +01:00

WM: Fix invalid memory access in wmTimer handling code.

Bastien Montagne referenced this issue

2023-09-18 18:19:52 +02:00

USD: Support armature and shape key export #111931

Bastien Montagne commented

2023-09-18 18:21:39 +02:00

This unfortunately needs to be re-opened, when massively generating reports from threaded code it still causes several issues. Will update description for specific info.

Bastien Montagne reopened this issue

2023-09-18 18:21:41 +02:00

Bastien Montagne commented

2023-09-18 18:22:23 +02:00

Actually better use a new task imho: #112537

Blender Bot added

and removed

labels 2023-09-18 18:22:40 +02:00