Skip to content

fix(python): guard dataset index reads after close - #999

Merged
ColinLeeo merged 1 commit into
apache:developfrom
ColinLeeo:codex/fix-mapped-dataset-index-close-guard
Oct 9, 2026
Merged

ColinLeeo merged 1 commit into
apache:developfrom
ColinLeeo:codex/fix-mapped-dataset-index-close-guard

Conversation

@ColinLeeo

@ColinLeeo ColinLeeo commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Reading a MappedDatasetIndex after it has been closed can raise TypeError: a bytes-like object is required, not 'NoneType' when unpacking the cleared mmap view. Accelerated lookups can instead raise AttributeError because the Cython lookup has been cleared. These exceptions hide the actual cause and prevent callers from reliably handling closed resources with except RuntimeError.

The same issue occurs when releasing the final runtime lease closes the index. For example:

first = TsFileDataFrame(dataset_dir, show_progress=False, use_index=True)
subset = first[:1]
name = str(subset.list_timeseries()[0])
first.close()
subset.close()
subset[name][:]

This change uses the existing _view is None state to reject reads with RuntimeError("Dataset Index is closed"). A shared _assert_open() guard covers record, string, count, and accelerated metadata lookup access, and prevents re-entering a closed index. Repeated calls to close() remain safe.

Regression tests cover:

  • Explicit close, context manager exit, and runtime teardown across the index read APIs.
  • Empty record ranges and iterators created or partially consumed before close.
  • The named subset read above after the final runtime lease is released.

Validation from python/:

  • python3.12 -m pytest -q tests/test_dataset_index.py tests/test_tsfile_dataset.py — 218 passed.
  • python3.12 -m black --check tsfile/dataset/index.py tests/test_dataset_index.py — passed with Black 26.3.1.
  • git diff --check — passed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The guard comprehensively covers index reads, and regression tests exercise the relevant closure paths.

0 open findings

What changed in this PR

Adds consistent closed-state handling for memory-mapped dataset indexes.

Changes:

  • Guards all index read paths with a clear RuntimeError.
  • Adds regression coverage for explicit, contextual, and runtime-driven closure.
File Description
python/​tsfile/​dataset/​index.py Adds shared open-state validation to index APIs.
python/​tests/​test_dataset_index.py Tests closed-index access and runtime teardown scenarios.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ColinLeeo
ColinLeeo merged commit d33e640 into apache:develop Oct 9, 2026
30 checks passed
@ColinLeeo
ColinLeeo deleted the codex/fix-mapped-dataset-index-close-guard branch October 9, 2026 04:13
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.

2 participants