From ef49d0b35b6da07ccfe30cf5a87d42a79ce3a711 Mon Sep 17 00:00:00 2001 From: Your Name Date: Sat, 13 Dec 2025 08:06:37 +0900 Subject: [PATCH] Format Issue #25 section to match existing Known Limitations format and add link to issue --- README.md | 161 ++++++++++-------------------------------------------- 1 file changed, 30 insertions(+), 131 deletions(-) diff --git a/README.md b/README.md index 4a8189f..7165608 100644 --- a/README.md +++ b/README.md @@ -324,137 +324,36 @@ This node is designed to work with: - [Issue #7](https://github.com/ussoewwin/ComfyUI-QwenImageLoraLoader/issues/7) - RES4LYF sampler device mismatch error - [Issue #8](https://github.com/ussoewwin/ComfyUI-QwenImageLoraLoader/issues/8) - RES4LYF sampler compatibility issue -### Issue #25: ComfyUI 0.4.0 Model Management Errors - -**Additional Information Regarding Issue #25** - -After the ComfyUI 0.4.0 update, multiple nodes (including this one in some environments) experienced errors such as `TypeError: 'NoneType' object is not callable` and `AttributeError: 'NoneType' object has no attribute`. In our environment, we resolved this by modifying ComfyUI's core `model_management.py`. Note that in our environment, these errors did not occur with this node (ComfyUI-QwenImageLoraLoader). Nunchaku library and ComfyUI-Nunchaku nodes are assumed to be using the latest versions. If errors persist even after applying the latest versions, modification of ComfyUI's core `model_management.py` may be necessary. - -#### Root Cause - -In ComfyUI 0.4.0, ComfyUI's core `model_management.py` lacks sufficient None checks, causing `TypeError` and `AttributeError` when accessing methods or attributes on objects that became `None` after models were unloaded or garbage collected. - -**Note: GC Changes in ComfyUI 0.4.0** - -Compared to ComfyUI 0.3.x, automatic model unloading occurs earlier, making the following flow more likely: - -``` -ModelPatcher → GC → weakref(None) -``` - -This also explains why the occurrence of the issue varies by user environment. - -#### The Essence of the Problem: Structural Issues in ComfyUI Core Model Management - -This problem is not a bug in individual nodes, but rather a structural issue in ComfyUI 0.4.0's model management (`model_management.py`). - -##### Technical Basis - -1. **Multiple Locations with "Missing None Checks"** - - This is not a bug in individual nodes, but the core main logic crashes when accessing attributes on `None`. - - The added fixes like `if model is None: continue` are defensive code that ComfyUI core should have in all paths. - -2. **Post-Weak-Reference GC Behavior Not Considered** - - The introduction of `LoadedModel._model = weakref.ref(ModelPatcher)` in ComfyUI 0.4.0 is a breaking change. - - When the weak reference target is garbage collected, it returns `None`, but this is not handled. - - Post-processing for the breaking update is incomplete. - -3. **Multiple Nodes Were Affected in a Chain Reaction** - - This is not a problem with nodes, but multiple nodes were affected in a chain reaction due to core behavior changes. - - Model loading/unloading, memory calculation, GPU/CPU offloading, and `ModelPatcher` lifecycle are all controlled by ComfyUI core. - -4. **All Fix Locations Are Core Responsibility Areas** - - The locations fixed (`model_memory`, `model_offloaded_memory`, `load_models_gpu`, `free_memory`, `model_unload`, `is_dead` checks, etc.) are all ComfyUI core functions. - - These are not areas that node developers should touch. - - The fact that all fix locations are core logic leaves no explanation other than a core defect. - -5. **Result of Applying Fixes** - - After applying None check fixes to ComfyUI's core `model_management.py` in our environment, similar errors were resolved. - - This demonstrates that the problem can be solved by adding defensive code that the core should have. - -#### Model Lifecycle and ModelPatcher Initialization Relationship - -##### Fact 1: Relationship between LoadedModel and ModelPatcher - -The `LoadedModel` class (lines 502-524 in ComfyUI's `model_management.py`) holds a weak reference to `ModelPatcher`: - -```python -def _set_model(self, model): - self._model = weakref.ref(model) # Weak reference to ModelPatcher - -@property -def model(self): - return self._model() # Returns None when garbage collected -``` - -##### Fact 2: ModelPatcher Initialization - -In the `__init__` of the `ModelPatcher` class (lines 215-237 in `model_patcher.py`), the `pinned` attribute is initialized: - -```python -def __init__(self, model, load_device, offload_device, size=0, weight_inplace_update=False): - # ... - self.pinned = set() # Line 237: Initialized -``` - -##### Fact 3: Fix Content in ComfyUI Core's model_management.py - -The fix in ComfyUI's core `model_management.py` now skips `LoadedModel` instances where `model` is `None`: - -- In `load_models_gpu()`, skips `LoadedModel` instances where `model` is `None` (lines 712, 727, 743) -- In `free_memory()`, excludes `LoadedModel` instances where `model` is `None` (line 646) - -##### Fact 4: Problem Before Fix - -- `LoadedModel` holds a weak reference to `ModelPatcher` -- When garbage collected, `LoadedModel.model` returns `None` -- Before the fix, methods were called on `LoadedModel` instances where `model` was `None`, causing errors - -##### Fact 5: Behavior After Fix - -- By skipping `LoadedModel` instances where `model` is `None`, errors do not occur -- Because errors do not occur, processing continues normally - -##### Fact 6: Why copy.deepcopy Fails - -`copy.deepcopy` fails because references to GC'd `ModelPatcher` instances remain in the dictionary being deepcopied. When these references are accessed, they return `None`, causing deepcopy to stop. - -##### Fact 7: Confirmation Items - -- After applying the fix, `copy.deepcopy` and `pinned` attribute errors do not occur in our environment -- Nunchaku library and ComfyUI-Nunchaku nodes are assumed to be using the latest versions, but this may still be insufficient -- While these errors did not occur with this node (ComfyUI-QwenImageLoraLoader) in our environment, the fix to ComfyUI's core `model_management.py` may have indirectly affected it, making errors less likely to occur - -#### Important Note: Not a Problem with Nunchaku Library - -This problem is not caused by the Nunchaku library's implementation: - -- Nunchaku's `model_config` and `ModelPatcher` itself are normal -- The problem is in the upstream = ComfyUI core's `model_management.py` GC processing - -#### Speculation (Items That May Be Environment-Dependent) - -- The fix allows `ModelPatcher` initialization to complete normally -- As a result, the `pinned` attribute is also properly initialized -- Accessing `self.pinned` in `__del__` does not cause errors - -#### Recommendations - -1. Update Nunchaku library and ComfyUI-Nunchaku nodes to the latest version (addresses `model_config` issues) -2. Consider applying None check fixes to ComfyUI's core `model_management.py` (may address the root cause) - -Technical details regarding fixes to ComfyUI's core `model_management.py` will be added later. - -This may also resolve `copy.deepcopy` and `pinned` attribute errors. - -#### Related Information - -`NoneType` errors in ComfyUI 0.4.x series have been reported sporadically in the following Issues: -- ComfyUI Issue #6590: `'NoneType' object has no attribute 'shape'` -- ComfyUI Issue #6600: `'NoneType' object is not callable` (Loader-related) -- ComfyUI Issue #6532: Crash after referencing models after model unload - -- **Related Issue**: [Issue #25](https://github.com/ussoewwin/ComfyUI-QwenImageLoraLoader/issues/25) - `AttributeError: 'NunchakuModelPatcher' object has no attribute 'pinned'` and deepcopy errors with `model_config` +### [Issue #25: ComfyUI 0.4.0 Model Management Errors](https://github.com/ussoewwin/ComfyUI-QwenImageLoraLoader/issues/25) +- **Status**: ⚠️ **Environment Dependent** - May require ComfyUI core fixes +- **Issue**: After the ComfyUI 0.4.0 update, multiple nodes (including this one in some environments) experienced errors such as `TypeError: 'NoneType' object is not callable` and `AttributeError: 'NoneType' object has no attribute`. In our environment, these errors did not occur with this node (ComfyUI-QwenImageLoraLoader), but we resolved similar errors by modifying ComfyUI's core `model_management.py`. +- **Root Cause**: In ComfyUI 0.4.0, ComfyUI's core `model_management.py` lacks sufficient None checks, causing `TypeError` and `AttributeError` when accessing methods or attributes on objects that became `None` after models were unloaded or garbage collected. This problem is not a bug in individual nodes, but rather a structural issue in ComfyUI 0.4.0's model management (`model_management.py`). +- **GC Changes in ComfyUI 0.4.0**: Compared to ComfyUI 0.3.x, automatic model unloading occurs earlier, making the following flow more likely: `ModelPatcher → GC → weakref(None)`. This also explains why the occurrence of the issue varies by user environment. +- **Technical Basis**: + 1. **Multiple Locations with "Missing None Checks"** - This is not a bug in individual nodes, but the core main logic crashes when accessing attributes on `None`. The added fixes like `if model is None: continue` are defensive code that ComfyUI core should have in all paths. + 2. **Post-Weak-Reference GC Behavior Not Considered** - The introduction of `LoadedModel._model = weakref.ref(ModelPatcher)` in ComfyUI 0.4.0 is a breaking change. When the weak reference target is garbage collected, it returns `None`, but this is not handled. Post-processing for the breaking update is incomplete. + 3. **Multiple Nodes Were Affected in a Chain Reaction** - This is not a problem with nodes, but multiple nodes were affected in a chain reaction due to core behavior changes. Model loading/unloading, memory calculation, GPU/CPU offloading, and `ModelPatcher` lifecycle are all controlled by ComfyUI core. + 4. **All Fix Locations Are Core Responsibility Areas** - The locations fixed (`model_memory`, `model_offloaded_memory`, `load_models_gpu`, `free_memory`, `model_unload`, `is_dead` checks, etc.) are all ComfyUI core functions. These are not areas that node developers should touch. The fact that all fix locations are core logic leaves no explanation other than a core defect. + 5. **Result of Applying Fixes** - After applying None check fixes to ComfyUI's core `model_management.py` in our environment, similar errors were resolved. This demonstrates that the problem can be solved by adding defensive code that the core should have. +- **Model Lifecycle and ModelPatcher Initialization Relationship**: + - **Fact 1: Relationship between LoadedModel and ModelPatcher** - The `LoadedModel` class (lines 502-524 in ComfyUI's `model_management.py`) holds a weak reference to `ModelPatcher`: `def _set_model(self, model): self._model = weakref.ref(model)` and `@property def model(self): return self._model()` which returns `None` when garbage collected. + - **Fact 2: ModelPatcher Initialization** - In the `__init__` of the `ModelPatcher` class (lines 215-237 in `model_patcher.py`), the `pinned` attribute is initialized: `self.pinned = set()` at line 237. + - **Fact 3: Fix Content in ComfyUI Core's model_management.py** - The fix in ComfyUI's core `model_management.py` now skips `LoadedModel` instances where `model` is `None`: In `load_models_gpu()`, skips `LoadedModel` instances where `model` is `None` (lines 712, 727, 743); In `free_memory()`, excludes `LoadedModel` instances where `model` is `None` (line 646). + - **Fact 4: Problem Before Fix** - `LoadedModel` holds a weak reference to `ModelPatcher`. When garbage collected, `LoadedModel.model` returns `None`. Before the fix, methods were called on `LoadedModel` instances where `model` was `None`, causing errors. + - **Fact 5: Behavior After Fix** - By skipping `LoadedModel` instances where `model` is `None`, errors do not occur. Because errors do not occur, processing continues normally. + - **Fact 6: Why copy.deepcopy Fails** - `copy.deepcopy` fails because references to GC'd `ModelPatcher` instances remain in the dictionary being deepcopied. When these references are accessed, they return `None`, causing deepcopy to stop. + - **Fact 7: Confirmation Items** - After applying the fix, `copy.deepcopy` and `pinned` attribute errors do not occur in our environment. Nunchaku library and ComfyUI-Nunchaku nodes are assumed to be using the latest versions, but this may still be insufficient. While these errors did not occur with this node (ComfyUI-QwenImageLoraLoader) in our environment, the fix to ComfyUI's core `model_management.py` may have indirectly affected it, making errors less likely to occur. +- **Important Note: Not a Problem with Nunchaku Library** - This problem is not caused by the Nunchaku library's implementation. Nunchaku's `model_config` and `ModelPatcher` itself are normal. The problem is in the upstream = ComfyUI core's `model_management.py` GC processing. +- **Speculation (Items That May Be Environment-Dependent)**: The fix allows `ModelPatcher` initialization to complete normally. As a result, the `pinned` attribute is also properly initialized. Accessing `self.pinned` in `__del__` does not cause errors. +- **Recommendations**: + 1. Update Nunchaku library and ComfyUI-Nunchaku nodes to the latest version (addresses `model_config` issues) + 2. Consider applying None check fixes to ComfyUI's core `model_management.py` (may address the root cause) +- **Note**: Technical details regarding fixes to ComfyUI's core `model_management.py` will be added later. This may also resolve `copy.deepcopy` and `pinned` attribute errors. +- **Related Issues**: + - [Issue #25](https://github.com/ussoewwin/ComfyUI-QwenImageLoraLoader/issues/25) - `AttributeError: 'NunchakuModelPatcher' object has no attribute 'pinned'` and deepcopy errors with `model_config` + - ComfyUI Issue #6590: `'NoneType' object has no attribute 'shape'` + - ComfyUI Issue #6600: `'NoneType' object is not callable` (Loader-related) + - ComfyUI Issue #6532: Crash after referencing models after model unload ## Changelog