You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ra...@apache.org on 2023/11/28 15:03:05 UTC
(arrow) 06/16: GH-38626: [Python] Fix segfault when PyArrow is imported at shutdown (#38637)
This is an automated email from the ASF dual-hosted git repository.
raulcd pushed a commit to branch maint-14.0.x
in repository https://gitbox.apache.org/repos/asf/arrow.git
commit 039582d941de331331cbed4ff85bbe44e0ea80e4
Author: Antoine Pitrou <an...@python.org>
AuthorDate: Tue Nov 14 14:25:29 2023 +0100
GH-38626: [Python] Fix segfault when PyArrow is imported at shutdown (#38637)
### Rationale for this change
Some C++ destructors may be called after the Python interpreter has ceased to exist.
If such a destructor tries to call back in the Python interpreter, for example by calling `Py_DECREF`, we get a crash.
### What changes are included in this PR?
Protect `OwnedRef` and `OwneRefNoGIL` destructors against decref'ing a Python object after Python finalization.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
No.
* Closes: #38626
Authored-by: Antoine Pitrou <an...@python.org>
Signed-off-by: Joris Van den Bossche <jo...@gmail.com>
---
python/pyarrow/src/arrow/python/common.h | 17 ++++++++++-------
python/pyarrow/tests/test_misc.py | 13 +++++++++++++
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/python/pyarrow/src/arrow/python/common.h b/python/pyarrow/src/arrow/python/common.h
index e36c0834fd..bc567ef78e 100644
--- a/python/pyarrow/src/arrow/python/common.h
+++ b/python/pyarrow/src/arrow/python/common.h
@@ -188,7 +188,12 @@ class ARROW_PYTHON_EXPORT OwnedRef {
return *this;
}
- ~OwnedRef() { reset(); }
+ ~OwnedRef() {
+ // GH-38626: destructor may be called after the Python interpreter is finalized.
+ if (Py_IsInitialized()) {
+ reset();
+ }
+ }
void reset(PyObject* obj) {
Py_XDECREF(obj_);
@@ -225,13 +230,11 @@ class ARROW_PYTHON_EXPORT OwnedRefNoGIL : public OwnedRef {
explicit OwnedRefNoGIL(PyObject* obj) : OwnedRef(obj) {}
~OwnedRefNoGIL() {
- // This destructor may be called after the Python interpreter is finalized.
- // At least avoid spurious attempts to take the GIL when not necessary.
- if (obj() == NULLPTR) {
- return;
+ // GH-38626: destructor may be called after the Python interpreter is finalized.
+ if (Py_IsInitialized() && obj() != NULLPTR) {
+ PyAcquireGIL lock;
+ reset();
}
- PyAcquireGIL lock;
- reset();
}
};
diff --git a/python/pyarrow/tests/test_misc.py b/python/pyarrow/tests/test_misc.py
index 9b9dfdd554..a48ac0c3cd 100644
--- a/python/pyarrow/tests/test_misc.py
+++ b/python/pyarrow/tests/test_misc.py
@@ -117,6 +117,19 @@ def test_runtime_info():
subprocess.check_call([sys.executable, "-c", code], env=env)
+def test_import_at_shutdown():
+ # GH-38626: importing PyArrow at interpreter shutdown would crash
+ code = """if 1:
+ import atexit
+
+ def import_arrow():
+ import pyarrow
+
+ atexit.register(import_arrow)
+ """
+ subprocess.check_call([sys.executable, "-c", code])
+
+
@pytest.mark.skipif(sys.platform == "win32",
reason="Path to timezone database is not configurable "
"on non-Windows platforms")