Summary
_read_cog_http calls _fetch_decode_cog_http_tiles(source, ...) then source.close() without a try/finally. If the fetch/decode step raises, the source is never closed.
Where
xrspatial/geotiff/_reader.py:1938-1940:
arr = _fetch_decode_cog_http_tiles(
source, header, ifd, max_pixels=max_pixels, window=window)
source.close()
The band-validation block immediately above (lines 1917-1936) is more careful: every raise path explicitly calls source.close() before raising. The bug is the gap between that careful path and the unguarded fetch/decode call.
Impact
_HTTPSource.close() is currently a no-op (the urllib3 PoolManager is shared module-level), so this is latent rather than active. The risk is structural: a future change that gives _HTTPSource actual resources to release will silently leak them on the error path.
Proposed fix
Wrap the fetch/decode and the post-processing block (band extract, orientation, photometric inversion) in try/finally with source.close() in finally. _HTTPSource.close() is idempotent, so there is no double-close concern with the earlier explicit closes on the validation paths.
Tests
Mock _fetch_decode_cog_http_tiles to raise, call _read_cog_http, and assert source.close() was called. A second test confirms the happy path still closes exactly once.
Summary
_read_cog_httpcalls_fetch_decode_cog_http_tiles(source, ...)thensource.close()without a try/finally. If the fetch/decode step raises, the source is never closed.Where
xrspatial/geotiff/_reader.py:1938-1940:The band-validation block immediately above (lines 1917-1936) is more careful: every raise path explicitly calls
source.close()before raising. The bug is the gap between that careful path and the unguarded fetch/decode call.Impact
_HTTPSource.close()is currently a no-op (the urllib3 PoolManager is shared module-level), so this is latent rather than active. The risk is structural: a future change that gives_HTTPSourceactual resources to release will silently leak them on the error path.Proposed fix
Wrap the fetch/decode and the post-processing block (band extract, orientation, photometric inversion) in
try/finallywithsource.close()infinally._HTTPSource.close()is idempotent, so there is no double-close concern with the earlier explicit closes on the validation paths.Tests
Mock
_fetch_decode_cog_http_tilesto raise, call_read_cog_http, and assertsource.close()was called. A second test confirms the happy path still closes exactly once.