Skip to content

fix: keep runtime locks outside the working directory - #311

Open
Liuchenbing-2026 wants to merge 1 commit into
modelscope:mainfrom
Liuchenbing-2026:fix/readonly-runtime-locks-20261013
Open

Liuchenbing-2026 wants to merge 1 commit into
modelscope:mainfrom
Liuchenbing-2026:fix/readonly-runtime-locks-20261013

Conversation

@Liuchenbing-2026

Copy link
Copy Markdown
Contributor

PR type

  • Bug Fix
  • New Feature
  • Document Updates
  • More Models or Datasets Support

PR information

Importing twinkle.utils.parallel currently creates .locks in the working directory, causing PermissionError when that directory is read-only.

Move runtime locks to an absolute path under TWINKLE_HOME, defaulting to <tempdir>/twinkle/<user>/.locks, and create the directory only when a processing lock or one-shot claim is used. Processes sharing this runtime directory coordinate across different working directories. Multi-host runs should set TWINKLE_HOME to a writable shared filesystem.

Addresses the read-only working-directory lock problem in #298. Add filesystem and subprocess regressions for both default/custom runtime paths, import without directory creation, cross-directory locking and claims, session rollover, and exception cleanup.

Experiment results

Validated on macOS arm64, CPython 3.12.13, datasets 5.1.0, filelock 4.1.1, and pytest 9.1.1:

  • The same regression tests on base ec8fd1f0660b09ed60938d02d2439533a5aa5c65: 5 failed, 1 passed, including real read-only-directory PermissionError failures.
  • On head 0de02dfe4103ef9033befdc72f90d1e4a1ce93b3: 6 passed, no skips.
  • All applicable changed-file pre-commit hooks passed.
python -m pytest tests/utils/test_parallel.py -q
pre-commit run --files src/twinkle/utils/parallel.py tests/utils/test_parallel.py

The tests load the real utility module directly and use real FileLock/filesystem/subprocess behavior. Read-only permission cases skip for root or non-POSIX environments. Validation covers this CPU utility; full training/SDK integration and hardware tests remain untested.

AI assistance: Codex assisted with the implementation and regression tests.

Use a per-user temporary directory, with TWINKLE_HOME as an override,
and create it only when locks are used. Add real filesystem and
cross-process regressions for read-only working directories.

Signed-off-by: liuchenbing <chenliumail@163.com>
@Liuchenbing-2026

Copy link
Copy Markdown
Contributor Author

@Yunnglin could you please review this focused fix for the read-only working-directory part of #298?

I reran the committed regression tests against the original base (ec8fd1f0660b09ed60938d02d2439533a5aa5c65), current main (9953ea1ebbbdbe59c3831702157d3c147ca0ee26), and this PR's unchanged head (0de02dfe4103ef9033befdc72f90d1e4a1ce93b3). The tests use the actual utility source, filesystem permissions, and separate processes:

Revision Result
Original base 5 failed, 1 passed
Current main 5 failed, 1 passed
PR head 6 passed, no skips

Rechecked on Linux/aarch64 with Python 3.12.13 and datasets 4.8.4, running as an unprivileged user so the read-only-directory cases really exercise PermissionError. Coverage includes lazy initialization, default/custom runtime directories, cross-directory locking/claims, session rollover, and exception cleanup.

This addresses only the lock-directory subproblem, not all of #298. Multi-host deployment and the full Tinker training workflow were not exercised in this rerun. Thanks!

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