BKE_reports: make the API thread-safe.

This commit makes using (most of) `BKE_report` API safe in
multi-threaded situation.

This is achieved by adding a `std::mutex` lock to the `ReportList`
struct (in a slightly convoluted way unfortunately, due to this being a
DNA struct). This lock is then used to make most operations on
`Reportlist` data thread-safe.

Note that while working on this, a few other minor issues aroze in
existing usages of Reportlist by the WM code, mainly the fact that
`wm_init_reports` and `wm_free_reports` were both useless:
  - init was called in a context where there is not yet any WM, so it
    was doing nothing.
  - free was called on a WM that would be later freed (as part of Main
    freeing), which would also call cleanup code for its `reports` data.
Both have been removed.

Further more, `wm_add_default` (which is the only place where a WM ID is
created) did not initialize properly it reports data, this has been
fixed.

This change is related to the wmJob thread-safety tasks and PRs (#112537,
!113548).

Pull Request: https://projects.blender.org/blender/blender/pulls/113561

source/blender/blenkernel/BKE_report.h

@@ -18,21 +18,47 @@
 extern "C" {
 #endif
 
-/* Reporting Information and Errors
+/** Reporting Information and Errors.
  *
- * These functions also accept NULL in case no error reporting
- * is needed. */
+ * These functions are thread-safe, unless otherwise specified.
+ *
+ * These functions also accept nullptr in case no error reporting is needed. The message are only
+ * printed to the console then. */
 
 /* Report structures are stored in DNA. */
 
+/** Initialize a #ReportList struct.
+ *
+ * \note: Not thread-safe, should only be called from the 'owner' thread of the report list.
+ */
 void BKE_reports_init(ReportList *reports, int flag);
 /**
- * Only frees the list \a reports.
+ * Fully release any internal resources used by this #ReportList, as acquired by #BKE_reports_init.
+ *
+ * Also calls #BKE_reports_clear. The given `reports` should not be used anymore unless it is
+ * re-initialized first.
+ *
+ * \note: Not thread-safe, should only be called from the current owner of the report list, once
+ * no other concurrent access is possible.
+ */
+void BKE_reports_free(ReportList *reports);
+
+/**
+ * Only frees the list of reports in given \a reports. Use #BKE_reports_free to fully cleanup all
+ * allocated resources.
+ *
  * To make displayed reports disappear, either remove window-manager reports
  * (#wmWindowManager.reports, or #CTX_wm_reports()), or use #WM_report_banners_cancel().
  */
 void BKE_reports_clear(ReportList *reports);
 
+/** Moves all reports from `reports_src` to `reports_dst`. */
+void BKE_reports_move_to_reports(ReportList *reports_dst, ReportList *reports_src);
+
+/** (Un)lock given `reports`, in case external code needs to access its data. */
+void BKE_reports_lock(ReportList *reports);
+void BKE_reports_unlock(ReportList *reports);
+
 void BKE_report(ReportList *reports, eReportType type, const char *message);
 void BKE_reportf(ReportList *reports, eReportType type, const char *format, ...)
     ATTR_PRINTF_FORMAT(3, 4);

source/blender/blenkernel/intern/report.cc

@@ -10,11 +10,13 @@
 #include <cstdarg>
 #include <cstdio>
 #include <cstring>
+#include <mutex>
 
 #include "MEM_guardedalloc.h"
 
 #include "BLI_blenlib.h"
 #include "BLI_dynstr.h"
+#include "BLI_listbase.h"
 #include "BLI_string_utils.h"
 #include "BLI_utildefines.h"
 
@@ -60,6 +62,20 @@ void BKE_reports_init(ReportList *reports, int flag)
   reports->storelevel = RPT_INFO;
   reports->printlevel = RPT_ERROR;
   reports->flag = flag;
+
+  reports->lock = MEM_new<std::mutex>(__func__);
+}
+
+void BKE_reports_free(ReportList *reports)
+{
+  if (!reports) {
+    return;
+  }
+
+  BKE_reports_clear(reports);
+
+  MEM_delete(reports->lock);
+  reports->lock = nullptr;
 }
 
 void BKE_reports_clear(ReportList *reports)
@@ -70,6 +86,8 @@ void BKE_reports_clear(ReportList *reports)
     return;
   }
 
+  std::scoped_lock lock(*reports->lock);
+
   report = static_cast<Report *>(reports->list.first);
 
   while (report) {
@@ -82,6 +100,28 @@ void BKE_reports_clear(ReportList *reports)
   BLI_listbase_clear(&reports->list);
 }
 
+void BKE_reports_lock(ReportList *reports)
+{
+  reports->lock->lock();
+}
+
+void BKE_reports_unlock(ReportList *reports)
+{
+  reports->lock->unlock();
+}
+
+void BKE_reports_move_to_reports(ReportList *reports_dst, ReportList *reports_src)
+{
+  BLI_assert(reports_dst);
+  if (!reports_src) {
+    return;
+  }
+
+  std::scoped_lock lock(*reports_src->lock, *reports_dst->lock);
+
+  BLI_movelisttolist(&reports_dst->list, &reports_src->list);
+}
+
 void BKE_report(ReportList *reports, eReportType type, const char *_message)
 {
   Report *report;
@@ -94,6 +134,8 @@ void BKE_report(ReportList *reports, eReportType type, const char *_message)
   }
 
   if (reports && (reports->flag & RPT_STORE) && (type >= reports->storelevel)) {
+    std::scoped_lock lock(*reports->lock);
+
     char *message_alloc;
     report = static_cast<Report *>(MEM_callocN(sizeof(Report), "Report"));
     report->type = type;
@@ -124,6 +166,8 @@ void BKE_reportf(ReportList *reports, eReportType type, const char *_format, ...
   }
 
   if (reports && (reports->flag & RPT_STORE) && (type >= reports->storelevel)) {
+    std::scoped_lock lock(*reports->lock);
+
     report = static_cast<Report *>(MEM_callocN(sizeof(Report), "Report"));
 
     va_start(args, _format);
@@ -145,6 +189,9 @@ static void reports_prepend_impl(ReportList *reports, const char *prepend)
 {
   /* Caller must ensure. */
   BLI_assert(reports && reports->list.first);
+
+  std::scoped_lock lock(*reports->lock);
+
   const size_t prefix_len = strlen(prepend);
   LISTBASE_FOREACH (Report *, report, &reports->list) {
     char *message = BLI_string_joinN(prepend, report->message);
@@ -193,6 +240,8 @@ void BKE_report_print_level_set(ReportList *reports, eReportType level)
     return;
   }
 
+  std::scoped_lock lock(*reports->lock);
+
   reports->printlevel = level;
 }
 
@@ -211,6 +260,8 @@ void BKE_report_store_level_set(ReportList *reports, eReportType level)
     return;
   }
 
+  std::scoped_lock lock(*reports->lock);
+
   reports->storelevel = level;
 }
 
@@ -223,6 +274,8 @@ char *BKE_reports_string(ReportList *reports, eReportType level)
     return nullptr;
   }
 
+  std::scoped_lock lock(*reports->lock);
+
   ds = BLI_dynstr_new();
   LISTBASE_FOREACH (Report *, report, &reports->list) {
     if (report->type >= level) {
@@ -274,6 +327,8 @@ void BKE_reports_print(ReportList *reports, eReportType level)
 
 Report *BKE_reports_last_displayable(ReportList *reports)
 {
+  std::scoped_lock lock(*reports->lock);
+
   LISTBASE_FOREACH_BACKWARD (Report *, report, &reports->list) {
     if (ELEM(report->type, RPT_ERROR, RPT_WARNING, RPT_INFO)) {
       return report;
@@ -286,6 +341,8 @@ Report *BKE_reports_last_displayable(ReportList *reports)
 bool BKE_reports_contain(ReportList *reports, eReportType level)
 {
   if (reports != nullptr) {
+    std::scoped_lock lock(*reports->lock);
+
     LISTBASE_FOREACH (Report *, report, &reports->list) {
       if (report->type >= level) {
         return true;
@@ -301,6 +358,8 @@ bool BKE_report_write_file_fp(FILE *fp, ReportList *reports, const char *header)
     fputs(header, fp);
   }
 
+  std::scoped_lock lock(*reports->lock);
+
   LISTBASE_FOREACH (Report *, report, &reports->list) {
     fprintf((FILE *)fp, "%s  # %s\n", report->message, report->typestr);
   }

source/blender/blenloader/tests/blendfile_loading_base_test.cc

@@ -120,7 +120,7 @@ bool BlendfileLoadingBaseTest::blendfile_load(const char *filepath)
   char abspath[FILE_MAX];
   BLI_path_join(abspath, sizeof(abspath), test_assets_dir.c_str(), filepath);
 
-  BlendFileReadReport bf_reports = {nullptr};
+  BlendFileReadReport bf_reports = {};
   bfile = BLO_read_from_file(abspath, BLO_READ_SKIP_NONE, &bf_reports);
   if (bfile == nullptr) {
     ADD_FAILURE() << "Unable to load file '" << filepath << "' from test assets dir '"
@@ -143,10 +143,6 @@ void BlendfileLoadingBaseTest::blendfile_free()
     return;
   }
 
-  wmWindowManager *wm = static_cast<wmWindowManager *>(bfile->main->wm.first);
-  if (wm != nullptr) {
-    wm_close_and_free(nullptr, wm);
-  }
   BLO_blendfiledata_free(bfile);
   bfile = nullptr;
 }

source/blender/editors/interface/interface_region_menu_popup.cc

@@ -566,6 +566,8 @@ void UI_popup_menu_reports(bContext *C, ReportList *reports)
     return;
   }
 
+  BKE_reports_lock(reports);
+
   LISTBASE_FOREACH (Report *, report, &reports->list) {
     int icon;
     const char *msg, *msg_next;
@@ -601,6 +603,8 @@ void UI_popup_menu_reports(bContext *C, ReportList *reports)
     } while ((msg = msg_next) && *msg);
   }
 
+  BKE_reports_unlock(reports);
+
   if (pup) {
     UI_popup_menu_end(C, pup);
   }

source/blender/editors/render/render_opengl.cc

@@ -1065,6 +1065,7 @@ static void write_result(TaskPool *__restrict pool, WriteTaskData *task_data)
     }
   }
   if (reports.list.first != nullptr) {
+    /* TODO: Should rather use new #BKE_reports_move_to_reports ? */
     BLI_spin_lock(&oglrender->reports_lock);
     for (Report *report = static_cast<Report *>(reports.list.first); report != nullptr;
          report = report->next)
@@ -1073,6 +1074,7 @@ static void write_result(TaskPool *__restrict pool, WriteTaskData *task_data)
     }
     BLI_spin_unlock(&oglrender->reports_lock);
   }
+  BKE_reports_free(&reports);
   if (!ok) {
     oglrender->pool_ok = false;
   }

source/blender/editors/space_file/space_file.cc

@@ -129,7 +129,7 @@ static void file_free(SpaceLink *sl)
   MEM_SAFE_FREE(sfile->params);
   MEM_SAFE_FREE(sfile->asset_params);
   if (sfile->runtime != nullptr) {
-    BKE_reports_clear(&sfile->runtime->is_blendfile_readable_reports);
+    BKE_reports_free(&sfile->runtime->is_blendfile_readable_reports);
   }
   MEM_SAFE_FREE(sfile->runtime);
 

source/blender/editors/space_image/image_ops.cc

@@ -2336,7 +2336,7 @@ bool ED_image_should_save_modified(const Main *bmain)
   uint modified_images_count = ED_image_save_all_modified_info(bmain, &reports);
   bool should_save = modified_images_count || !BLI_listbase_is_empty(&reports.list);
 
-  BKE_reports_clear(&reports);
+  BKE_reports_free(&reports);
 
   return should_save;
 }

source/blender/makesdna/DNA_windowmanager_types.h

@@ -14,6 +14,13 @@
 
 #include "DNA_ID.h"
 
+#ifdef __cplusplus
+#  include <mutex>
+using std_mutex_type = std::mutex;
+#else
+#  define std_mutex_type void
+#endif
+
 /* Defined here: */
 
 struct wmWindow;
@@ -98,6 +105,9 @@ typedef struct ReportList {
   int flag;
   char _pad[4];
   struct wmTimer *reporttimer;
+
+  /** Mutex for thread-safety, runtime only. */
+  std_mutex_type *lock;
 } ReportList;
 
 /* Timer custom-data to control reports display. */
@@ -162,6 +172,7 @@ typedef struct wmWindowManager {
    * \note keep in sync with `notifier_queue` adding/removing elements must also update this set.
    */
   struct GSet *notifier_queue_set;
+  void *_pad1;
 
   /** Information and error reports. */
   struct ReportList reports;

source/blender/modifiers/intern/MOD_datatransfer.cc

@@ -227,6 +227,8 @@ static Mesh *modify_mesh(ModifierData *md, const ModifierEvalContext *ctx, Mesh
         ctx->object, (ModifierData *)dtmd, "Enable 'Auto Smooth' in Object Data Properties");
   }
 
+  BKE_reports_free(&reports);
+
   return result;
 }
 

source/blender/python/intern/bpy_capi_utils.cc

@@ -33,7 +33,7 @@ short BPy_reports_to_error(ReportList *reports, PyObject *exception, const bool
   report_str = BKE_reports_string(reports, RPT_ERROR);
 
   if (clear == true) {
-    BKE_reports_clear(reports);
+    BKE_reports_free(reports);
   }
 
   if (report_str) {

source/blender/python/intern/bpy_capi_utils.h

@@ -18,7 +18,10 @@ extern "C" {
 
 struct ReportList;
 
-/* error reporting */
+/** Error reporting: convert BKE_report (#ReportList) reports into python errors.
+ *
+ * \param clear: When `true`, #BKE_reports_free is called on the given `reports`, which should
+ * then be considered as 'freed' data and not used anymore. */
 short BPy_reports_to_error(struct ReportList *reports, PyObject *exception, bool clear);
 /**
  * A version of #BKE_report_write_file_fp that uses Python's stdout.

source/blender/python/intern/bpy_interface_run.cc

@@ -281,11 +281,9 @@ static bool bpy_run_string_impl(bContext *C,
 
     ReportList *wm_reports = CTX_wm_reports(C);
     if (wm_reports) {
-      BLI_movelisttolist(&wm_reports->list, &reports.list);
-    }
-    else {
-      BKE_reports_clear(&reports);
+      BKE_reports_move_to_reports(wm_reports, &reports);
     }
+    BKE_reports_free(&reports);
   }
   else {
     Py_DECREF(retval);

source/blender/python/intern/bpy_library_load.cc

@@ -575,6 +575,8 @@ static PyObject *bpy_lib_exit(BPy_Library *self, PyObject * /*args*/)
   BKE_blendfile_link_append_context_free(lapp_context);
   BKE_main_id_tag_all(bmain, LIB_TAG_PRE_EXISTING, false);
 
+  BKE_reports_free(&self->reports);
+
   Py_RETURN_NONE;
 }
 

source/blender/python/intern/bpy_library_write.cc

@@ -187,17 +187,18 @@ static PyObject *bpy_lib_write(BPy_PropertyRNA *self, PyObject *args, PyObject *
 
   if (retval) {
     BKE_reports_print(&reports, RPT_ERROR_ALL);
-    BKE_reports_clear(&reports);
     ret = Py_None;
     Py_INCREF(ret);
   }
   else {
-    if (BPy_reports_to_error(&reports, PyExc_IOError, true) == 0) {
+    if (BPy_reports_to_error(&reports, PyExc_IOError, false) == 0) {
       PyErr_SetString(PyExc_IOError, "Unknown error writing library data");
     }
     ret = nullptr;
   }
 
+  BKE_reports_free(&reports);
+
 finally:
 
   /* clear all flags for ID's added to the store (may run on error too) */

source/blender/python/intern/bpy_operator.cc

@@ -263,6 +263,7 @@ static PyObject *pyop_call(PyObject * /*self*/, PyObject *args)
 
       BKE_reports_clear(reports);
       if ((reports->flag & RPT_FREE) == 0) {
+        BKE_reports_free(reports);
         MEM_freeN(reports);
       }
       else {

source/blender/python/intern/bpy_rna.cc

@@ -8792,7 +8792,8 @@ static int bpy_class_call(bContext *C, PointerRNA *ptr, FunctionRNA *func, Param
       BKE_reports_init(&reports_temp, reports->flag | RPT_PRINT_HANDLED_BY_OWNER);
       reports_temp.storelevel = reports->storelevel;
       BPy_errors_to_report(&reports_temp);
-      BLI_movelisttolist(&reports->list, &reports_temp.list);
+      BKE_reports_move_to_reports(reports, &reports_temp);
+      BKE_reports_free(&reports_temp);
     }
 
     /* Also print in the console for Python. */
@@ -9006,11 +9007,12 @@ static PyObject *pyrna_register_class(PyObject * /*self*/, PyObject *py_class)
     if (!has_error) {
       BPy_reports_write_stdout(&reports, error_prefix);
     }
-    BKE_reports_clear(&reports);
     if (has_error) {
+      BKE_reports_free(&reports);
       return nullptr;
     }
   }
+  BKE_reports_free(&reports);
 
   /* Python errors validating are not converted into reports so the check above will fail.
    * the cause for returning nullptr will be printed as an error */

source/blender/windowmanager/gizmo/intern/wm_gizmo_group.cc

@@ -102,7 +102,7 @@ void wm_gizmogroup_free(bContext *C, wmGizmoGroup *gzgroup)
 #endif
 
   if (gzgroup->reports && (gzgroup->reports->flag & RPT_FREE)) {
-    BKE_reports_clear(gzgroup->reports);
+    BKE_reports_free(gzgroup->reports);
     MEM_freeN(gzgroup->reports);
   }
 

source/blender/windowmanager/intern/wm.cc

@@ -293,7 +293,7 @@ void WM_operator_free(wmOperator *op)
   }
 
   if (op->reports && (op->reports->flag & RPT_FREE)) {
-    BKE_reports_clear(op->reports);
+    BKE_reports_free(op->reports);
     MEM_freeN(op->reports);
   }
 
@@ -344,7 +344,7 @@ void WM_operator_type_set(wmOperator *op, wmOperatorType *ot)
 
 static void wm_reports_free(wmWindowManager *wm)
 {
-  BKE_reports_clear(&wm->reports);
+  BKE_reports_free(&wm->reports);
   WM_event_timer_remove(wm, nullptr, wm->reports.reporttimer);
 }
 
@@ -524,6 +524,8 @@ void wm_add_default(Main *bmain, bContext *C)
   WorkSpace *workspace;
   WorkSpaceLayout *layout = BKE_workspace_layout_find_global(bmain, screen, &workspace);
 
+  BKE_reports_init(&wm->reports, RPT_STORE);
+
   CTX_wm_manager_set(C, wm);
   win = wm_window_new(bmain, wm, nullptr, false);
   win->scene = CTX_data_scene(C);

source/blender/windowmanager/intern/wm_event_system.cc

@@ -935,7 +935,7 @@ static void wm_add_reports(ReportList *reports)
     wmWindowManager *wm = static_cast<wmWindowManager *>(G_MAIN->wm.first);
 
     /* Add reports to the global list, otherwise they are not seen. */
-    BLI_movelisttolist(&wm->reports.list, &reports->list);
+    BKE_reports_move_to_reports(&wm->reports, reports);
 
     WM_report_banner_show(wm, nullptr);
   }
@@ -950,7 +950,7 @@ void WM_report(eReportType type, const char *message)
 
   wm_add_reports(&reports);
 
-  BKE_reports_clear(&reports);
+  BKE_reports_free(&reports);
 }
 
 void WM_reportf(eReportType type, const char *format, ...)
@@ -2831,12 +2831,7 @@ static eHandlerActionFlag wm_handler_fileselect_do(bContext *C,
           BKE_report_print_level_set(handler->op->reports, RPT_WARNING);
           UI_popup_menu_reports(C, handler->op->reports);
 
-          /* XXX: copied from #wm_operator_finished(). */
-          /* Add reports to the global list, otherwise they are not seen. */
-          BLI_movelisttolist(&CTX_wm_reports(C)->list, &handler->op->reports->list);
-
-          /* More hacks, since we meddle with reports, banner display doesn't happen automatic. */
-          WM_report_banner_show(CTX_wm_manager(C), CTX_wm_window(C));
+          wm_add_reports(handler->op->reports);
 
           CTX_wm_window_set(C, win_prev);
           CTX_wm_area_set(C, area_prev);

source/blender/windowmanager/intern/wm_files.cc

@@ -4278,7 +4278,7 @@ static uiBlock *block_create__close_file_dialog(bContext *C, ARegion *region, vo
     has_extra_checkboxes = true;
   }
 
-  BKE_reports_clear(&reports);
+  BKE_reports_free(&reports);
 
   uiItemS_ex(layout, has_extra_checkboxes ? 2.0f : 4.0f);
 

source/blender/windowmanager/intern/wm_init_exit.cc

@@ -137,19 +137,6 @@ CLG_LOGREF_DECLARE_GLOBAL(WM_LOG_MSGBUS_SUB, "wm.msgbus.sub");
 
 static void wm_init_scripts_extensions_once(bContext *C);
 
-static void wm_init_reports(bContext *C)
-{
-  ReportList *reports = CTX_wm_reports(C);
-
-  BLI_assert(!reports || BLI_listbase_is_empty(&reports->list));
-
-  BKE_reports_init(reports, RPT_STORE);
-}
-static void wm_free_reports(wmWindowManager *wm)
-{
-  BKE_reports_clear(&wm->reports);
-}
-
 static bool wm_start_with_console = false;
 
 void WM_init_state_start_with_console_set(bool value)
@@ -265,10 +252,6 @@ void WM_init(bContext *C, int argc, const char **argv)
   BKE_icons_init(BIFICONID_LAST_STATIC);
   BKE_preview_images_init();
 
-  /* Reports can't be initialized before the window-manager,
-   * but keep before file reading, since that may report errors */
-  wm_init_reports(C);
-
   WM_msgbus_types_init();
 
   /* Studio-lights needs to be init before we read the home-file,
@@ -619,11 +602,6 @@ void WM_exit_ex(bContext *C, const bool do_python_exit, const bool do_user_exit_
   ED_preview_restart_queue_free();
   ED_assetlist_storage_exit();
 
-  if (wm) {
-    /* Before BKE_blender_free! - since the ListBases get freed there. */
-    wm_free_reports(wm);
-  }
-
   SEQ_clipboard_free(); /* `sequencer.cc` */
   BKE_tracking_clipboard_free();
   BKE_mask_clipboard_free();

source/creator/creator_args.cc

@@ -1815,7 +1815,7 @@ static int arg_handle_render_frame(int argc, const char **argv, void *data)
         }
       }
       RE_SetReports(re, nullptr);
-      BKE_reports_clear(&reports);
+      BKE_reports_free(&reports);
       MEM_freeN(frame_range_arr);
       return 1;
     }
@@ -1842,7 +1842,7 @@ static int arg_handle_render_animation(int /*argc*/, const char ** /*argv*/, voi
     RE_RenderAnim(
         re, bmain, scene, nullptr, nullptr, scene->r.sfra, scene->r.efra, scene->r.frame_step);
     RE_SetReports(re, nullptr);
-    BKE_reports_clear(&reports);
+    BKE_reports_free(&reports);
   }
   else {
     fprintf(stderr, "\nError: no blend loaded. cannot use '-a'.\n");
@@ -2180,7 +2180,7 @@ static bool handle_load_file(bContext *C, const char *filepath_arg, const bool l
   BKE_reports_init(&reports, RPT_PRINT);
   WM_file_autoexec_init(filepath);
   const bool success = WM_file_read(C, filepath, &reports);
-  BKE_reports_clear(&reports);
+  BKE_reports_free(&reports);
 
   if (success) {
     if (G.background) {