saturday06 / VRM-Addon-for-Blender

VRM Importer, Exporter and Utilities for Blender 2.93 to 4.2
https://vrm-addon-for-blender.info
MIT License
1.3k stars 142 forks source link

Bug report: unstable reloading system, sys.modules should not be used and is causing severe crashes with every single other addon #506

Open Valery-AA opened 3 months ago

Valery-AA commented 3 months ago

sys.modules should not be touched in any way and makes the current way the addon works hang python in blender, with errors from every single addon that, the module is gone and cannot be reimported

source 1: https://justus.science/blog/2015/04/19/sys.modules-is-dangerous.html source 2: https://github.com/Valery-AA/AlxBPYCodeReferences/blob/main/Addon%20Register%20And%20Reload/Reloading%20Multi-File%20Addons.md

additionally i have made available to the community an module auto-loader to solve this problem

auto-loader:

import bpy

##################################################
############# ALX MODULE AUTO-LOADER #############
import os
import importlib

folder_blacklist = ["__pycache__"]
file_blacklist = ["__init__.py"]

addon_folders = list([__path__[0]])
addon_folders.extend( [os.path.join(__path__[0], folder_name) for folder_name in os.listdir(__path__[0]) if ( os.path.isdir( os.path.join(__path__[0], folder_name) ) ) and (folder_name not in folder_blacklist) ] )

addon_files = [[folder_path, file_name[0:-3]] for folder_path in addon_folders for file_name in os.listdir(folder_path) if (file_name not in file_blacklist) and (file_name.endswith(".py"))]

for folder_file_batch in addon_files:
    if (os.path.basename(folder_file_batch[0]) == os.path.basename(__path__[0])):
        file = folder_file_batch[1]

        if (file not in locals()):
            import_line = f"from . import {file}"
            exec(import_line)
        else:
            reload_line = f"{file} = importlib.reload({file})"
            exec(reload_line)

    else:
        if (os.path.basename(folder_file_batch[0]) != os.path.basename(__path__[0])):
            file = folder_file_batch[1]

            if (file not in locals()):
                import_line = f"from . {os.path.basename(folder_file_batch[0])} import {file}"
                exec(import_line)
            else:
                reload_line = f"{file} = importlib.reload({file})"
                exec(reload_line)

import inspect

class_blacklist = []

bpy_class_object_list = tuple(bpy_class[1] for bpy_class in inspect.getmembers(bpy.types, inspect.isclass) if (bpy_class not in class_blacklist))
alx_class_object_list = tuple(alx_class[1] for file_batch in addon_files for alx_class in inspect.getmembers(eval(file_batch[1]), inspect.isclass) if issubclass(alx_class[1], bpy_class_object_list) and (not issubclass(alx_class[1], bpy.types.WorkSpaceTool)))

AlxClassQueue = alx_class_object_list

def AlxRegisterClassQueue():
    for AlxClass in AlxClassQueue:
        try:
            bpy.utils.register_class(AlxClass)
        except:
            try:
                bpy.utils.unregister_class(AlxClass)
                bpy.utils.register_class(AlxClass)
            except:
                pass
def AlxUnregisterClassQueue():
    for AlxClass in AlxClassQueue:
        try:
            bpy.utils.unregister_class(AlxClass)
        except:
            print("Can't Unregister", AlxClass)

##################################################

simply add the code above below the bl_info struct

simply add alxregister/alxunregister like this to blender's register/unregister functions

def register():
    AlxRegisterClassQueue()

def unregister():
    AlxUnregisterClassQueue()
saturday06 commented 3 months ago

Thank you for the report. I would like to disable it by default.

This function may be necessary during development, so enable it if the environment variable BLENDER_VRM_DEVEMOPMENT_MODE=yes.

@tdw46 As the original author of the code which edits sys.modules , do you have any opinions on this?

saturday06 commented 3 months ago

The changes have been reflected in 2.20.60, which has just been released.

tdw46 commented 3 months ago

@tdw46 As the original author of the code which edits sys.modules , do you have any opinions on this?

Thanks for the ping to make me aware of this! I actually had no clue that touching the sys modules like this could cause such an issue. Agreed to keep them behind a dev flag in the env variables. Good call.

saturday06 commented 3 months ago

Thank you for your opinion! I will proceed with the implementation of 2.20.60.