[Shizoqua]

Issue resolved by this Pull Request:
Closes #3910

Summary

VLM engine cleanup can run during interpreter shutdown after module globals have been cleared. In that state, _log may be None, causing cleanup to raise while trying to log completion.

Fix

Guard cleanup logging when the module logger is unavailable across the affected VLM inference engines.

Tests

Added model-free regression coverage for the affected VLM engine cleanup paths with _log cleared.

The regression test fails before the fix with 5 failures and passes after the fix with 5 passed.

ruff check and ruff format --check also pass for the changed files.

Checklist:

• Documentation has been updated, if necessary.
• Examples have been added, if necessary.
• Tests have been added, if necessary.

[github-actions]

✅ DCO Check Passed

Thanks @Shizoqua, all your commits are properly signed off. 🎉

[mergify]

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

• title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?(!)?:

[codecov]

Codecov Report

❌ Patch coverage is 14.28571% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...erence_engines/vlm/api_openai_compatible_engine.py	0.00%	2 Missing ⚠️
docling/models/inference_engines/vlm/mlx_engine.py	0.00%	2 Missing ⚠️
...ocling/models/inference_engines/vlm/vllm_engine.py	0.00%	2 Missing ⚠️
...escription/picture_description_vlm_engine_model.py	0.00%	2 Missing ⚠️
...ing/models/stages/vlm_convert/vlm_convert_model.py	0.00%	2 Missing ⚠️
...models/inference_engines/vlm/auto_inline_engine.py	50.00%	0 Missing and 1 partial ⚠️
...odels/inference_engines/vlm/transformers_engine.py	50.00%	0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[Shizoqua]

@dolfim-ibm fixed a typo in the guard lines that was breaking the import, tests actually run and pass now.

[github-actions]

AI triage (advisory)

Checked commit 7e71929700. The analysis read the diff only. It did not run code from this pull request.

This pull request prevents errors during interpreter shutdown by checking if the module logger is available before logging. It adds guards to VLM engines and includes unit tests for the cleanup paths.

Topic labels added: bug, vlm-pipeline, tests, error-handling.

CI safety: no concern found. A maintainer must still approve CI.

1 related pull request(s) in the same code

PR	State	Signal	Assessment
#4642	open	changes the same code	related: PR 4642 touches the same areas and contains a similar guard in vlm_convert_model.py, but it focuses on MLX generation issues.

_{The ai:* labels are advisory. They do not approve CI and they do not add tests:full. A new push runs this check again.}

[dolfim-ibm]

The _log is None state only happens when __del__ runs during interpreter finalization, after CPython has cleared the module dicts (underscore names go first, modules in reverse import order). Guarding every log call treats the symptom and adds None checks to cleanup() methods that are also called at normal runtime.

A cleaner fix is to replace __del__ with weakref.finalize. If the model is collected at runtime, the callback runs then; if it is still alive at exit, the callback runs from an atexit hook, which fires before module teardown, so _log and every other global are still valid. No guards needed.

Sketch for VlmConvertModel (same for PictureDescriptionVlmEngineModel):

import weakref

# in __init__, after self.engine is created:
self._finalizer = weakref.finalize(self, _cleanup_engine, self.engine)


# module level, must not capture self or the model is never collected
def _cleanup_engine(engine: BaseVlmEngine) -> None:
    try:
        engine.cleanup()
    except Exception as e:
        _log.warning(f"Error cleaning up engine: {e}")

Then drop __del__ and keep the engine cleanup() methods unchanged. This also removes the hasattr(self, "engine") probe: the finalizer only exists if __init__ got far enough to create the engine.

It also makes the test simpler and closer to the real failure. The current test builds objects with object.__new__, sets private attributes per class, and patches _log, so it mostly checks that the guard exists. A subprocess test that keeps a model in a module global, exits, and asserts that stderr has no Error cleaning up engine covers the actual regression.

The same __del__ pattern is used in other places, which have the same shutdown exposure:

• docling/models/stages/code_formula/code_formula_vlm_model.py (currently patched with the same _log is not None guard)
• docling/models/stages/ocr/tesseract_ocr_model.py
• docling/models/stages/ocr/kserve_v2_ocr_model.py
• docling/models/stages/chart_extraction/granite_vision.py
• docling/models/inference_engines/common/kserve_v2_grpc.py
• docling/models/inference_engines/image_classification/api_kserve_v2_engine.py
• docling/models/inference_engines/object_detection/api_kserve_v2_engine.py

Would you be up for moving these to weakref.finalize as well, either in this PR or in a follow-up? Doing it in one place would keep the cleanup pattern consistent across the models and engines.

[dolfim-ibm]

@Shizoqua see the comment above, let's address it correctly for all the engines. Thanks for bringing it up