Skip to content

BUG: Close cached raster files before interpreter shutdown - #936

Closed
fallenmi wants to merge 1 commit into
corteva:masterfrom
fallenmi:agent/fix-open-rasterio-shutdown-cleanup
Closed

BUG: Close cached raster files before interpreter shutdown#936
fallenmi wants to merge 1 commit into
corteva:masterfrom
fallenmi:agent/fix-open-rasterio-shutdown-cleanup

Conversation

@fallenmi

Copy link
Copy Markdown

Summary

  • track open rasterio file handles without extending their lifetime
  • close remaining cached files before xarray disables final cleanup during interpreter shutdown
  • avoid waiting on files that are actively locked by another thread
  • add subprocess regressions for the reported macOS shutdown output and concurrent cache edge cases

Root cause

open_rasterio creates a cached .rio accessor cycle, so unclosed raster-backed arrays can survive until interpreter shutdown. xarray removes CachingFileManager.__del__ in its own atexit hook; rasterio handles that remain open after the hook can then emit empty sys.excepthook errors during teardown. Concurrent manager finalizers can also leave files in xarray's cache after the manager objects themselves are gone.

The weak-key registry follows the file handles rather than retaining their managers. Normal close and cache eviction remove entries automatically. At process exit, the callback closes only files whose associated lock can be acquired immediately, avoiding both concurrent close and shutdown hangs.

Closes #929.

Validation

  • exact current-master reproducer: RED with 5/5 teardown errors; GREEN with no stderr after this change
  • focused shutdown, concurrency, pickle, caching, and chunk lifecycle tests: 7 passed
  • integration I/O tests supported by the local GDAL build: 92 passed, 1 skipped, 1 xfailed, 1 xpassed; 8 HDF4-dependent cases deselected because the local Rasterio/GDAL wheel lacks HDF4 support
  • Black 23.12.1, isort 5.13.2, Flake8 6.1.0, Pylint, mypy on rioxarray/_io.py, and git diff --check: passed

AI assistance disclosure

OpenAI Codex reproduced the issue, prepared the implementation and tests, audited visible overlap and contribution-policy constraints, and ran the validation above under the account owner's authorization. The AI-assisted nature of this contribution is disclosed for maintainer review.

@snowman2

Copy link
Copy Markdown
Member

I would prefer an implementation that allows xarray to handle it instead of a custom implementation. acquire_context may or may not be useful here.

@fallenmi

Copy link
Copy Markdown
Author

Thanks — I checked acquire_context against both the declared minimum xarray 2026.2 and current xarray 2026.7. It closes a newly opened file only when the context body raises; it intentionally does not close after a successful acquisition. That covers the constructor-exception path, but not the successful unclosed teardown or threaded-orphan paths in #929.

I also retested clean current rioxarray master with Rasterio 1.5.1: the exact #929 reproducer still emits one shutdown error per open, and the constructor-exception, pickle, and threaded-orphan variants also reproduce. A direct unclosed rasterio.open and an explicit DataArray.close() are clean.

I will not defend the current custom registry. Would you prefer that I close #936 in favor of an upstream xarray lifecycle issue, or narrow the rioxarray guidance to require/document explicit close or context-manager use?

@fallenmi

Copy link
Copy Markdown
Author

Closing this draft. The current weak-registry/atexit patch is not a maintainable fit for the requested xarray-managed design.

CachingFileManager.acquire_context() closes a newly opened file only when the context body raises; it intentionally leaves a successful acquisition open. Xarray also deliberately removes CachingFileManager.__del__ during interpreter shutdown because cleanup at that stage may be unsafe (pydata/xarray#7880). Those semantics do not cover #929 successful-unclosed or threaded-orphan paths, so a bounded rioxarray use of acquire_context cannot replace the custom registry.

Explicit DataArray.close() and context-manager use remain clean. Leaving #929 open for an upstream lifecycle decision or documentation of explicit close; no replacement cross-project code is proposed here.

@fallenmi fallenmi closed this Aug 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Empty Exception for each open_rasterio call

2 participants