WIP: Add a mechanism to abort a blend file reading on critical error. #105085

Closed
Bastien Montagne wants to merge 1 commits from mont29:F-abort-file-reading into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
10 changed files with 191 additions and 36 deletions

View File

@ -138,6 +138,13 @@ typedef struct Main {
*/
bool is_locked_for_linking;
/**
* When set, indicates that an unrecoverable error/data corruption was detected.
* Should only be set by readfile code, and used by upper-level code (typically #setup_app_data)
* to cancel a file reading operation.
*/
bool is_read_invalid;
/**
* True if this main is the 'GMAIN' of current Blender.
*

View File

@ -476,6 +476,13 @@ void BKE_blendfile_read_setup_ex(bContext *C,
const bool startup_update_defaults,
const char *startup_app_template)
{
if (bfd->main->is_read_invalid) {
BKE_reports_prepend(reports->reports,
"File could not be read, critical data corruption detected");
BLO_blendfiledata_free(bfd);
return;
}
if (startup_update_defaults) {
if ((params->skip_flags & BLO_READ_SKIP_DATA) == 0) {
BLO_update_defaults_startup_blend(bfd->main, startup_app_template);
@ -503,6 +510,10 @@ struct BlendFileData *BKE_blendfile_read(const char *filepath,
}
BlendFileData *bfd = BLO_read_from_file(filepath, eBLOReadSkip(params->skip_flags), reports);
if (bfd && bfd->main->is_read_invalid) {
BLO_blendfiledata_free(bfd);
bfd = nullptr;
}
if (bfd) {
handle_subversion_warning(bfd->main, reports);
}
@ -519,6 +530,10 @@ struct BlendFileData *BKE_blendfile_read_from_memory(const void *filebuf,
{
BlendFileData *bfd = BLO_read_from_memory(
filebuf, filelength, eBLOReadSkip(params->skip_flags), reports);
if (bfd && bfd->main->is_read_invalid) {
BLO_blendfiledata_free(bfd);
bfd = nullptr;
}
if (bfd) {
/* Pass. */
}
@ -535,6 +550,10 @@ struct BlendFileData *BKE_blendfile_read_from_memfile(Main *bmain,
{
BlendFileData *bfd = BLO_read_from_memfile(
bmain, BKE_main_blendfile_path(bmain), memfile, params, reports);
if (bfd && bfd->main->is_read_invalid) {
BLO_blendfiledata_free(bfd);
bfd = nullptr;
}
if (bfd) {
/* Removing the unused workspaces, screens and wm is useless here, setup_app_data will switch
* those lists with the ones from old bmain, which freeing is much more efficient than

View File

@ -40,6 +40,14 @@ Main *BKE_main_new()
void BKE_main_free(Main *mainvar)
{
/* In case this is called on a 'split' list of mains.
*
* Should not happen in typical usages, but can occur e.g. if a file reading is aborted on an
* exception. */
if (mainvar->next) {
BKE_main_free(mainvar->next);
}
/* also call when reading a file, erase all, etc */
ListBase *lbarray[INDEX_ID_MAX];
int a;

View File

@ -288,6 +288,26 @@ struct LinkNode *BLO_blendhandle_get_linkable_groups(BlendHandle *bh);
*/
void BLO_blendhandle_close(BlendHandle *bh);
/** Mark the given Main (and the 'root' local one in case of lib-split Mains) as invalid, and
* generate an error report containing given `message`. */
void BLO_read_invalidate_message(BlendHandle *bh, struct Main *bmain, const char *message);
/**
* BLI_assert-like macro to check a condition, and if `false`, fail the whole .blend reading
* process by marking the Main data-base as invalid, and returning provided `_ret_value`.
*
* NOTE: About usages:
* - #BLI_assert should be used when the error is considered as a bug, but there is some code to
* recover from it and produce a valid Main data-base.
* - #BLO_read_assert_message should be used when the error is not considered as recoverable.
*/
#define BLO_read_assert_message(_check_expr, _ret_value, _bh, _bmain, _message) \
if (_check_expr) { \
BLO_read_invalidate_message((_bh), (_bmain), (_message)); \
return _ret_value; \
} \
(void)0
/** \} */
#define BLO_GROUP_MAX 32

View File

@ -385,6 +385,13 @@ void BLO_blendhandle_close(BlendHandle *bh)
blo_filedata_free(fd);
}
void BLO_read_invalidate_message(BlendHandle *bh, Main *bmain, const char *message)
{
FileData *fd = reinterpret_cast<FileData *>(bh);
blo_readfile_invalidate(fd, bmain, message);
}
/**********/
BlendFileData *BLO_read_from_file(const char *filepath,

View File

@ -320,6 +320,10 @@ static void add_main_to_main(Main *mainvar, Main *from)
ListBase *lbarray[INDEX_ID_MAX], *fromarray[INDEX_ID_MAX];
int a;
if (from->is_read_invalid) {
mainvar->is_read_invalid = true;
}
set_listbasepointers(mainvar, lbarray);
a = set_listbasepointers(from, fromarray);
while (a--) {
@ -340,6 +344,9 @@ void blo_join_main(ListBase *mainlist)
}
while ((tojoin = mainl->next)) {
if (tojoin->is_read_invalid) {
mainl->is_read_invalid = true;
}
add_main_to_main(mainl, tojoin);
BLI_remlink(mainlist, tojoin);
BKE_main_free(tojoin);
@ -548,6 +555,21 @@ static Main *blo_find_main(FileData *fd, const char *filepath, const char *relab
return m;
}
void blo_readfile_invalidate(FileData *fd, Main *bmain, const char *message)
{
/* Tag given bmain, and 'root 'local' main one (in case given one is a library one) as invalid.
*/
bmain->is_read_invalid = true;
for (; bmain->prev != nullptr; bmain = bmain->prev)
;
bmain->is_read_invalid = true;
BLO_reportf_wrap(fd->reports,
RPT_ERROR,
"A critical error happened (the blend file is likely corrupted): %s",
message);
}
/** \} */
/* -------------------------------------------------------------------- */
@ -3582,7 +3604,7 @@ static void do_versions(FileData *fd, Library *lib, Main *main)
main->is_locked_for_linking = false;
}
static void do_versions_after_linking(Main *main, ReportList *reports)
static void do_versions_after_linking(FileData *fd, Main *main)
{
CLOG_INFO(&LOG,
2,
@ -3595,13 +3617,27 @@ static void do_versions_after_linking(Main *main, ReportList *reports)
/* Don't allow versioning to create new data-blocks. */
main->is_locked_for_linking = true;
do_versions_after_linking_250(main);
do_versions_after_linking_260(main);
do_versions_after_linking_270(main);
do_versions_after_linking_280(main, reports);
do_versions_after_linking_290(main, reports);
do_versions_after_linking_300(main, reports);
do_versions_after_linking_cycles(main);
if (!main->is_read_invalid) {
do_versions_after_linking_250(main);
}
if (!main->is_read_invalid) {
do_versions_after_linking_260(main);
}
if (!main->is_read_invalid) {
do_versions_after_linking_270(main);
}
if (!main->is_read_invalid) {
do_versions_after_linking_280(fd, main);
}
if (!main->is_read_invalid) {
do_versions_after_linking_290(fd, main);
}
if (!main->is_read_invalid) {
do_versions_after_linking_300(fd, main);
}
if (!main->is_read_invalid) {
mont29 marked this conversation as resolved

I suggest to write it like this instead of returning early and having main->is_locked_for_linking = false duplicated.

if (!main->is_read_invalid) {
  do_versions_after_linking_280(fd, main);
}
if (!main->is_read_invalid) {
 do_versions_after_linking_290(fd, main);
}
...

And to do this for every do_versions, even if not needed now I think there is some chance of this getting overlooked in the future.

I suggest to write it like this instead of returning early and having `main->is_locked_for_linking = false` duplicated. ``` if (!main->is_read_invalid) { do_versions_after_linking_280(fd, main); } if (!main->is_read_invalid) { do_versions_after_linking_290(fd, main); } ... ``` And to do this for every `do_versions`, even if not needed now I think there is some chance of this getting overlooked in the future.
do_versions_after_linking_cycles(main);
}
main->is_locked_for_linking = false;
}
@ -3810,6 +3846,9 @@ static BHead *read_userdef(BlendFileData *bfd, FileData *fd, BHead *bhead)
static void blo_read_file_checks(Main *bmain)
{
#ifndef NDEBUG
BLI_assert(bmain->next == nullptr);
BLI_assert(!bmain->is_read_invalid);
LISTBASE_FOREACH (wmWindowManager *, wm, &bmain->wm) {
LISTBASE_FOREACH (wmWindow *, win, &wm->windows) {
/* This pointer is deprecated and should always be nullptr. */
@ -3820,23 +3859,11 @@ static void blo_read_file_checks(Main *bmain)
UNUSED_VARS_NDEBUG(bmain);
}
BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
static void blo_read_file_internal_do(FileData *fd, const char *filepath, BlendFileData *bfd)
{
BHead *bhead = blo_bhead_first(fd);
BlendFileData *bfd;
ListBase mainlist = {nullptr, nullptr};
if (fd->flags & FD_FLAGS_IS_MEMFILE) {
CLOG_INFO(&LOG_UNDO, 2, "UNDO: read step");
}
bfd = static_cast<BlendFileData *>(MEM_callocN(sizeof(BlendFileData), "blendfiledata"));
bfd->main = BKE_main_new();
bfd->main->versionfile = fd->fileversion;
bfd->type = BLENFILETYPE_BLEND;
if ((fd->skip_flags & BLO_READ_SKIP_DATA) == 0) {
BLI_addtail(&mainlist, bfd->main);
fd->mainlist = &mainlist;
@ -3914,6 +3941,10 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
bhead = read_libblock(fd, bfd->main, bhead, LIB_TAG_LOCAL, false, nullptr);
}
}
if (bfd->main->is_read_invalid) {
return;
}
}
/* do before read_libraries, but skip undo case */
@ -3927,6 +3958,10 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
}
}
if (bfd->main->is_read_invalid) {
return;
}
if ((fd->skip_flags & BLO_READ_SKIP_DATA) == 0) {
fd->reports->duration.libraries = PIL_check_seconds_timer();
read_libraries(fd, &mainlist);
@ -3953,7 +3988,7 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
blo_split_main(&mainlist, bfd->main);
LISTBASE_FOREACH (Main *, mainvar, &mainlist) {
BLI_assert(mainvar->versionfile != 0);
do_versions_after_linking(mainvar, fd->reports->reports);
do_versions_after_linking(fd, mainvar);
}
blo_join_main(&mainlist);
@ -3963,6 +3998,10 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
BKE_main_id_refcount_recompute(bfd->main, false);
}
if (bfd->main->is_read_invalid) {
return;
}
/* After all data has been read and versioned, uses LIB_TAG_NEW. Theoretically this should
* not be calculated in the undo case, but it is currently needed even on undo to recalculate
* a cache. */
@ -4004,6 +4043,24 @@ BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
/* Sanity checks. */
blo_read_file_checks(bfd->main);
}
BlendFileData *blo_read_file_internal(FileData *fd, const char *filepath)
{
BlendFileData *bfd;
if (fd->flags & FD_FLAGS_IS_MEMFILE) {
CLOG_INFO(&LOG_UNDO, 2, "UNDO: read step");
}
bfd = static_cast<BlendFileData *>(MEM_callocN(sizeof(BlendFileData), "blendfiledata"));
bfd->main = BKE_main_new();
bfd->main->versionfile = short(fd->fileversion);
bfd->type = BLENFILETYPE_BLEND;
blo_read_file_internal_do(fd, filepath, bfd);
return bfd;
}
@ -4170,10 +4227,8 @@ static ID *is_yet_read(FileData *fd, Main *mainvar, BHead *bhead)
/** \name Library Linking (expand pointers)
* \{ */
static void expand_doit_library(void *fdhandle, Main *mainvar, void *old)
static void expand_doit_library_do(FileData *fd, Main *mainvar, void *old)
{
FileData *fd = static_cast<FileData *>(fdhandle);
BHead *bhead = find_bhead(fd, old);
if (bhead == nullptr) {
return;
@ -4289,6 +4344,15 @@ static void expand_doit_library(void *fdhandle, Main *mainvar, void *old)
}
}
static void expand_doit_library(void *fdhandle, Main *mainvar, void *old)
{
FileData *fd = static_cast<FileData *>(fdhandle);
if (!mainvar->is_read_invalid) {
expand_doit_library_do(fd, mainvar, old);
}
}
static BLOExpandDoitCallback expand_doit;
static void expand_id(BlendExpander *expander, ID *id);
@ -4432,7 +4496,16 @@ ID *BLO_library_link_named_part(Main *mainl,
const LibraryLink_Params *params)
{
FileData *fd = (FileData *)(*bh);
return link_named_part(mainl, fd, idcode, name, params->flag);
ID *ret_id = nullptr;
if (!mainl->is_read_invalid) {
ret_id = link_named_part(mainl, fd, idcode, name, params->flag);
}
if (mainl->is_read_invalid) {
return nullptr;
}
return ret_id;
}
/* common routine to append/link something from a library */
@ -4562,6 +4635,10 @@ static void library_link_end(Main *mainl, FileData **fd, const int flag)
mainvar = static_cast<Main *>((*fd)->mainlist->first);
mainl = nullptr; /* blo_join_main free's mainl, can't use anymore */
if (mainvar->is_read_invalid) {
return;
}
lib_link_all(*fd, mainvar);
after_liblink_merged_bmain_process(mainvar);
@ -4582,9 +4659,13 @@ static void library_link_end(Main *mainl, FileData **fd, const int flag)
* or they will go again through do_versions - bad, very bad! */
split_main_newid(mainvar, main_newid);
do_versions_after_linking(main_newid, (*fd)->reports->reports);
do_versions_after_linking(*fd, main_newid);
add_main_to_main(mainvar, main_newid);
if (mainvar->is_read_invalid) {
return;
}
}
blo_join_main((*fd)->mainlist);
@ -4631,9 +4712,12 @@ static void library_link_end(Main *mainl, FileData **fd, const int flag)
void BLO_library_link_end(Main *mainl, BlendHandle **bh, const LibraryLink_Params *params)
{
FileData *fd = (FileData *)(*bh);
library_link_end(mainl, &fd, params->flag);
*bh = (BlendHandle *)fd;
FileData *fd = reinterpret_cast<FileData *>(*bh);
if (!mainl->is_read_invalid) {
library_link_end(mainl, &fd, params->flag);
*bh = reinterpret_cast<BlendHandle *>(fd);
}
}
void *BLO_library_read_struct(FileData *fd, BHead *bh, const char *blockname)

View File

@ -219,9 +219,9 @@ void blo_do_versions_cycles(struct FileData *fd, struct Library *lib, struct Mai
void do_versions_after_linking_250(struct Main *bmain);
void do_versions_after_linking_260(struct Main *bmain);
void do_versions_after_linking_270(struct Main *bmain);
void do_versions_after_linking_280(struct Main *bmain, struct ReportList *reports);
void do_versions_after_linking_290(struct Main *bmain, struct ReportList *reports);
void do_versions_after_linking_300(struct Main *bmain, struct ReportList *reports);
void do_versions_after_linking_280(struct FileData *fd, struct Main *bmain);
void do_versions_after_linking_290(struct FileData *fd, struct Main *bmain);
void do_versions_after_linking_300(struct FileData *fd, struct Main *bmain);
void do_versions_after_linking_cycles(struct Main *bmain);
/**
@ -232,6 +232,10 @@ void do_versions_after_linking_cycles(struct Main *bmain);
*/
void *blo_read_get_new_globaldata_address(struct FileData *fd, const void *adr);
/* Mark the Main data as invalid (.blend file reading should be aborted ASAP, and the already read
* data should be discarded). Also add an error report to `fd` including given `message`. */
void blo_readfile_invalidate(struct FileData *fd, struct Main *bmain, const char *message);
#ifdef __cplusplus
}
#endif

View File

@ -1171,7 +1171,7 @@ static void do_version_fcurve_hide_viewport_fix(struct ID *UNUSED(id),
fcu->rna_path = BLI_strdupn("hide_viewport", 13);
}
void do_versions_after_linking_280(Main *bmain, ReportList *UNUSED(reports))
void do_versions_after_linking_280(FileData *fd, Main *bmain)
{
bool use_collection_compat_28 = true;
@ -1241,6 +1241,12 @@ void do_versions_after_linking_280(Main *bmain, ReportList *UNUSED(reports))
if (!MAIN_VERSION_ATLEAST(bmain, 280, 0)) {
for (bScreen *screen = bmain->screens.first; screen; screen = screen->id.next) {
BLO_read_assert_message(screen->scene == NULL,
,
(BlendHandle *)fd,
bmain,
"No Screen data-block should ever have a NULL `scene` pointer");
/* same render-layer as do_version_workspaces_after_lib_link will activate,
* so same layer as BKE_view_layer_default_view would return */
ViewLayer *layer = screen->scene->view_layers.first;

View File

@ -410,7 +410,7 @@ static void version_node_socket_duplicate(bNodeTree *ntree,
}
}
void do_versions_after_linking_290(Main *bmain, ReportList * /*reports*/)
void do_versions_after_linking_290(FileData * /*fd*/, Main *bmain)
{
if (!MAIN_VERSION_ATLEAST(bmain, 290, 1)) {
/* Patch old grease pencil modifiers material filter. */

View File

@ -939,7 +939,7 @@ static void version_geometry_nodes_primitive_uv_maps(bNodeTree &ntree)
}
}
void do_versions_after_linking_300(Main *bmain, ReportList * /*reports*/)
void do_versions_after_linking_300(FileData * /*fd*/, Main *bmain)
{
if (MAIN_VERSION_ATLEAST(bmain, 300, 0) && !MAIN_VERSION_ATLEAST(bmain, 300, 1)) {
/* Set zero user text objects to have a fake user. */