Skip to content

[python] Keep RESTTokenFileIO usable in a forked child - #10556

Merged
JingsongLi merged 1 commit into
apache:masterfrom
sundapeng:fix-rest-file-io-fork-safety
Oct 14, 2026
Merged

JingsongLi merged 1 commit into
apache:masterfrom
sundapeng:fix-rest-file-io-fork-safety

Conversation

@sundapeng

@sundapeng sundapeng commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

Purpose

Follow-up to #10552. RESTTokenFileIO now looks up and creates FileIOs under one process-wide lock. If the process forks while another thread holds that lock, for example while that thread creates a FileIO, the child inherits the lock in its held state. The child's first file_io() call then blocks forever, whatever table it is for. Before #10552 the lock was per object, so only a child that used that same object could block.

The per-object lock and the per-table token lock have the same problem when a fork happens during a token refresh. That was already the case before #10552.

This pull request handles these locks the way the logging module handles its locks:

  • The cache lock is held across fork (os.register_at_fork). A fork waits for an in-progress lookup or FileIO creation, so the child sees the cache in a consistent state.
  • The child gets a new cache lock, new token locks, and a new lock for every RESTTokenFileIO object, which are tracked in a WeakSet.

Python 3.6 has no os.register_at_fork and behaves as before.

Tests

  • rest_token_file_io_test.py
    • test_fork_while_another_thread_creates_file_io: another thread is creating a FileIO when the process forks. The child looks up another table and finds the FileIO that was being created already cached.
    • test_fork_while_another_thread_refreshes_token: another thread is refreshing a token, so it holds the object's lock and the table's token lock, when the process forks. The child uses that object and a new object of the same table.
    • The child gets a 10 s alarm, so a blocked child fails the test instead of hanging it. Both tests fail on master. Removing any one part of the change (the before-fork hook, the new cache lock, the new token locks, or the new per-object locks) fails at least one of them.
  • The full Python suite with CI's selection (Python 3.11) has no failure that master does not also have; the remaining local failures are missing optional dependencies on both.
  • The test file passes on Python 3.11 and 3.13. On Python 3.6 the two new tests are skipped, and CI's Python 3.6 selection gives the same results as on master.
  • flake8 with dev/cfg.ini passes.

Verified with forks and JindoSDK

A probe forks children that use RESTTokenFileIO while another thread is busy in it. It uses local tables, because a forked child cannot use pyjindo once the parent has used it, with or without this change.

Scenario before #10552 master this PR
Fork while another thread creates a FileIO (3 runs) 0 / 3 children blocked 3 / 3 0 / 3
Fork 40 times while another thread keeps creating FileIOs (3 runs) 0 / 120 120 / 120 0 / 120

The pyjindo e2e scenarios of #10552 (pyjindosdk 6.10.4 against a local OSS-compatible server: streams open across token refreshes, concurrent readers, Avro and Parquet table reads and writes, injected 503s and dropped connections, normal and atexit exits, spawn children) were rerun with this change, and all pass.

API and Format

No.

Documentation

No.

Since apache#10552, RESTTokenFileIO creates FileIOs under one process-wide
lock. If the process forks while another thread holds that lock, for
example while that thread creates a FileIO, the child inherits the lock
in its held state and its first file_io() call blocks forever, whatever
table it is for. The per-object lock and the per-table token lock, held
while a token is refreshed, have had the same problem since before
apache#10552.

Do what the logging module does: hold the cache lock across fork, so
the child gets the cache in a consistent state, and give the child new
token locks and per-object locks. Python 3.6 has no os.register_at_fork
and behaves as before.
@JingsongLi

Copy link
Copy Markdown
Contributor

+1

@JingsongLi
JingsongLi merged commit 881a1bb into apache:master Oct 14, 2026
14 of 16 checks passed
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