This is an automated email from the ASF dual-hosted git repository.
JingsongLi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/paimon.git
The following commit(s) were added to refs/heads/master by this push:
new 1a465b7dbf [python] Fix import deadlock: lazy-resolve package exports,
drop custom locks (#9267)
1a465b7dbf is described below
commit 1a465b7dbfcac271f85a78449b3ce017ded259a2
Author: XiaoHongbo <[email protected]>
AuthorDate: Tue Aug 18 00:11:25 2026 +0800
[python] Fix import deadlock: lazy-resolve package exports, drop custom
locks (#9267)
---
paimon-python/pypaimon/__init__.py | 16 +++---
paimon-python/pypaimon/common/options/__init__.py | 34 ++++++++++--
.../pypaimon/common/options/core_options.py | 2 +-
paimon-python/pypaimon/common/options/options.py | 2 +-
paimon-python/pypaimon/data/__init__.py | 30 +++++++++--
paimon-python/pypaimon/globalindex/__init__.py | 16 +++---
paimon-python/pypaimon/tag/__init__.py | 16 +++---
.../pypaimon/tests/concurrent_import_test.py | 60 ++++++++++++++++++++++
8 files changed, 137 insertions(+), 39 deletions(-)
diff --git a/paimon-python/pypaimon/__init__.py
b/paimon-python/pypaimon/__init__.py
index 3aa61b4c95..0a26944944 100644
--- a/paimon-python/pypaimon/__init__.py
+++ b/paimon-python/pypaimon/__init__.py
@@ -17,7 +17,6 @@
import importlib
import sys
-import threading
if sys.version_info[:2] < (3, 8):
try:
@@ -53,18 +52,15 @@ _LAZY_EXPORTS = {
"SQLContext": ("pypaimon_rust.datafusion", "SQLContext"),
}
-# Serialize first-time imports: racing threads can otherwise acquire module
-# locks in opposite orders and fail with _DeadlockError.
-_LAZY_IMPORT_LOCK = threading.RLock()
-
+# Resolution stays unlocked: submodules import these names at their top
+# level, so a lock here would deadlock against Python's module locks.
def __getattr__(name):
target = _LAZY_EXPORTS.get(name)
if target is None:
raise AttributeError(
"module 'pypaimon' has no attribute {}".format(name))
- with _LAZY_IMPORT_LOCK:
- if name not in globals():
- module = importlib.import_module(target[0])
- globals()[name] = getattr(module, target[1])
- return globals()[name]
+ module = importlib.import_module(target[0])
+ value = getattr(module, target[1])
+ globals()[name] = value
+ return value
diff --git a/paimon-python/pypaimon/common/options/__init__.py
b/paimon-python/pypaimon/common/options/__init__.py
index 071ee21638..c96caa805b 100644
--- a/paimon-python/pypaimon/common/options/__init__.py
+++ b/paimon-python/pypaimon/common/options/__init__.py
@@ -15,10 +15,15 @@
# specific language governing permissions and limitations
# under the License.
-from .config_option import ConfigOption, Description
-from .config_options import ConfigOptions
-from .options import Options
-from .core_options import CoreOptions
+import importlib
+import sys
+
+if sys.version_info[:2] < (3, 7):
+ # Module-level __getattr__ is unavailable before Python 3.7.
+ from .config_option import ConfigOption, Description
+ from .config_options import ConfigOptions
+ from .options import Options
+ from .core_options import CoreOptions
__all__ = [
'ConfigOption',
@@ -27,3 +32,24 @@ __all__ = [
'Options',
'CoreOptions'
]
+
+_MODULE_BY_EXPORT = {
+ 'ConfigOption': 'pypaimon.common.options.config_option',
+ 'Description': 'pypaimon.common.options.config_option',
+ 'ConfigOptions': 'pypaimon.common.options.config_options',
+ 'Options': 'pypaimon.common.options.options',
+ 'CoreOptions': 'pypaimon.common.options.core_options',
+}
+
+
+# This package is imported by name from many modules that its own eager
+# exports pull in, so eager imports here deadlock concurrent importers.
+def __getattr__(name):
+ module_name = _MODULE_BY_EXPORT.get(name)
+ if module_name is None:
+ raise AttributeError(
+ "module 'pypaimon.common.options' has no attribute {}".format(
+ name))
+ value = getattr(importlib.import_module(module_name), name)
+ globals()[name] = value
+ return value
diff --git a/paimon-python/pypaimon/common/options/core_options.py
b/paimon-python/pypaimon/common/options/core_options.py
index e2cc0ddef2..7cde748d64 100644
--- a/paimon-python/pypaimon/common/options/core_options.py
+++ b/paimon-python/pypaimon/common/options/core_options.py
@@ -24,7 +24,7 @@ from typing import Dict, List, Optional
from urllib.parse import urlparse
from pypaimon.common.memory_size import MemorySize
-from pypaimon.common.options import Options
+from pypaimon.common.options.options import Options
from pypaimon.common.options.config_option import ConfigOption
from pypaimon.common.options.config_options import ConfigOptions
from pypaimon.common.options.options_utils import OptionsUtils
diff --git a/paimon-python/pypaimon/common/options/options.py
b/paimon-python/pypaimon/common/options/options.py
index f6145c4303..d81458d980 100644
--- a/paimon-python/pypaimon/common/options/options.py
+++ b/paimon-python/pypaimon/common/options/options.py
@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.
-from pypaimon.common.options import ConfigOption
+from pypaimon.common.options.config_option import ConfigOption
from pypaimon.common.options.options_utils import OptionsUtils
diff --git a/paimon-python/pypaimon/data/__init__.py
b/paimon-python/pypaimon/data/__init__.py
index 88fc282dbd..c0211f0460 100644
--- a/paimon-python/pypaimon/data/__init__.py
+++ b/paimon-python/pypaimon/data/__init__.py
@@ -15,9 +15,14 @@
# specific language governing permissions and limitations
# under the License.
-from pypaimon.data.timestamp import Timestamp
-from pypaimon.data.decimal import Decimal
-from pypaimon.data.variant_path import variant_get, variant_replace
+import importlib
+import sys
+
+if sys.version_info[:2] < (3, 7):
+ # Module-level __getattr__ is unavailable before Python 3.7.
+ from pypaimon.data.timestamp import Timestamp
+ from pypaimon.data.decimal import Decimal
+ from pypaimon.data.variant_path import variant_get, variant_replace
__all__ = [
'Timestamp',
@@ -25,3 +30,22 @@ __all__ = [
'variant_get',
'variant_replace',
]
+
+_MODULE_BY_EXPORT = {
+ 'Timestamp': 'pypaimon.data.timestamp',
+ 'Decimal': 'pypaimon.data.decimal',
+ 'variant_get': 'pypaimon.data.variant_path',
+ 'variant_replace': 'pypaimon.data.variant_path',
+}
+
+
+# Eager sibling imports here let one thread hold this package's module lock
+# while waiting on a sibling that another thread is already importing.
+def __getattr__(name):
+ module_name = _MODULE_BY_EXPORT.get(name)
+ if module_name is None:
+ raise AttributeError(
+ "module 'pypaimon.data' has no attribute {}".format(name))
+ value = getattr(importlib.import_module(module_name), name)
+ globals()[name] = value
+ return value
diff --git a/paimon-python/pypaimon/globalindex/__init__.py
b/paimon-python/pypaimon/globalindex/__init__.py
index 548dcab433..b0fee5ea03 100644
--- a/paimon-python/pypaimon/globalindex/__init__.py
+++ b/paimon-python/pypaimon/globalindex/__init__.py
@@ -17,7 +17,6 @@
import importlib
import sys
-import threading
if sys.version_info[:2] < (3, 7):
# Module-level __getattr__ is unavailable before Python 3.7.
@@ -107,18 +106,15 @@ _MODULE_BY_EXPORT = {
'Range': 'pypaimon.utils.range',
}
-# Eager exports would cycle: index_file_meta initializes this package while
-# create_global_index imports it back. The lock serializes racing imports.
-_LAZY_IMPORT_LOCK = threading.RLock()
-
+# Lazy resolution breaks the eager cycle where index_file_meta initializes
+# this package while create_global_index imports it back. Python's own
+# module locks make this thread-safe; an extra lock here would deadlock.
def __getattr__(name):
module_name = _MODULE_BY_EXPORT.get(name)
if module_name is None:
raise AttributeError(
"module 'pypaimon.globalindex' has no attribute {}".format(name))
- with _LAZY_IMPORT_LOCK:
- if name not in globals():
- module = importlib.import_module(module_name)
- globals()[name] = getattr(module, name)
- return globals()[name]
+ value = getattr(importlib.import_module(module_name), name)
+ globals()[name] = value
+ return value
diff --git a/paimon-python/pypaimon/tag/__init__.py
b/paimon-python/pypaimon/tag/__init__.py
index 8f7cc7f5b7..c3555d56e6 100644
--- a/paimon-python/pypaimon/tag/__init__.py
+++ b/paimon-python/pypaimon/tag/__init__.py
@@ -17,7 +17,6 @@
import importlib
import sys
-import threading
if sys.version_info[:2] < (3, 7):
# Module-level __getattr__ is unavailable before Python 3.7.
@@ -31,18 +30,15 @@ _MODULE_BY_EXPORT = {
"TagManager": "pypaimon.tag.tag_manager",
}
-# Eager sibling imports let threads lock tag and tag_manager in opposite
-# orders and fail with _DeadlockError. The lock serializes racing imports.
-_LAZY_IMPORT_LOCK = threading.RLock()
-
+# Lazy resolution avoids the eager sibling imports that let threads lock
+# tag and tag_manager in opposite orders. Python's own module locks make
+# this thread-safe; an extra lock here would deadlock against them.
def __getattr__(name):
module_name = _MODULE_BY_EXPORT.get(name)
if module_name is None:
raise AttributeError(
"module 'pypaimon.tag' has no attribute {}".format(name))
- with _LAZY_IMPORT_LOCK:
- if name not in globals():
- module = importlib.import_module(module_name)
- globals()[name] = getattr(module, name)
- return globals()[name]
+ value = getattr(importlib.import_module(module_name), name)
+ globals()[name] = value
+ return value
diff --git a/paimon-python/pypaimon/tests/concurrent_import_test.py
b/paimon-python/pypaimon/tests/concurrent_import_test.py
index 624a8c1a4c..9989763355 100644
--- a/paimon-python/pypaimon/tests/concurrent_import_test.py
+++ b/paimon-python/pypaimon/tests/concurrent_import_test.py
@@ -155,6 +155,66 @@ if errors:
"attempt {}: {}".format(attempt, result.stdout),
)
+ @unittest.skipIf(
+ sys.version_info[:2] < (3, 7),
+ "module-level lazy attributes require Python 3.7+",
+ )
+ def test_concurrent_leaf_imports_and_top_level_export(self):
+ # Racing leaf imports against top-level exports deadlocks whenever a
+ # package init eagerly imports siblings that import the package back.
+ script = r"""
+import sys
+import threading
+
+sys.setswitchinterval(1e-6)
+statements = [
+ "import pypaimon.manifest.schema.data_file_meta",
+ "import pypaimon.manifest.schema.simple_stats",
+ "from pypaimon import Schema",
+ "from pypaimon import CatalogFactory",
+ "import pypaimon.multimodal.connection",
+ "import pypaimon.read.query_auth_split",
+ "import pypaimon.write.table_delete",
+ "import pypaimon.index.index_file_meta",
+]
+barrier = threading.Barrier(len(statements))
+errors = []
+
+
+def resolve(statement):
+ try:
+ barrier.wait()
+ exec(statement)
+ except BaseException as error:
+ errors.append(repr(error))
+
+
+threads = [
+ threading.Thread(target=resolve, args=(statement,))
+ for statement in statements
+]
+for thread in threads:
+ thread.start()
+for thread in threads:
+ thread.join(15)
+if errors:
+ print("\n".join(sorted(set(errors))))
+ raise SystemExit(1)
+"""
+ for attempt in range(8):
+ result = subprocess.run(
+ [sys.executable, "-c", script],
+ env=os.environ.copy(),
+ stdout=subprocess.PIPE,
+ stderr=subprocess.STDOUT,
+ universal_newlines=True,
+ timeout=30,
+ )
+ self.assertEqual(
+ 0, result.returncode,
+ "attempt {}: {}".format(attempt, result.stdout),
+ )
+
def test_fresh_import_of_cycle_prone_leaf_modules(self):
# Each module must import cleanly as a process's first pypaimon
# import, without a package init pulling a circular chain.
