From a06d6f4e07cdc8e4a00cc491c92c3724d4ab0df3 Mon Sep 17 00:00:00 2001 From: Lydia Duncan Date: Tue, 12 Nov 2024 12:52:04 -0800 Subject: [PATCH 1/2] Add a test locking in the behavior I was seeing with unusual import paths When importing a Python library via the local directory structure instead of the PYTHONPATH, we were getting an assertion failure at the end of the Python file. This was because we'd intentionally cleaned up the Chapel runtime, but the __init__.py file had also registered a call to do so and the cleanup function wasn't re-entrant. Add a test that demonstrated the failure mode ---- Signed-off-by: Lydia Duncan --- .../python/unusualDirStructure/.gitignore | 1 + .../python/unusualDirStructure/COMPOPTS | 1 + .../python/unusualDirStructure/PREDIFF | 25 ++++++++++++++++ .../interop/python/unusualDirStructure/SKIPIF | 14 +++++++++ .../python/unusualDirStructure/myLoc.notest | 0 .../unusualDirStructure/skipif_helper.py | 14 +++++++++ .../python/unusualDirStructure/testing.chpl | 30 +++++++++++++++++++ .../unusualDirStructure/testing.cleanfiles | 1 + .../unusualDirStructure/testing.compopts | 1 + .../python/unusualDirStructure/testing.good | 3 ++ .../python/unusualDirStructure/testing.noexec | 0 .../unusualDirStructure/testing.prediff | 9 ++++++ .../python/unusualDirStructure/use_testing.py | 9 ++++++ 13 files changed, 108 insertions(+) create mode 100644 test/interop/python/unusualDirStructure/.gitignore create mode 100644 test/interop/python/unusualDirStructure/COMPOPTS create mode 100755 test/interop/python/unusualDirStructure/PREDIFF create mode 100755 test/interop/python/unusualDirStructure/SKIPIF create mode 100644 test/interop/python/unusualDirStructure/myLoc.notest create mode 100644 test/interop/python/unusualDirStructure/skipif_helper.py create mode 100644 test/interop/python/unusualDirStructure/testing.chpl create mode 100644 test/interop/python/unusualDirStructure/testing.cleanfiles create mode 100644 test/interop/python/unusualDirStructure/testing.compopts create mode 100644 test/interop/python/unusualDirStructure/testing.good create mode 100644 test/interop/python/unusualDirStructure/testing.noexec create mode 100755 test/interop/python/unusualDirStructure/testing.prediff create mode 100644 test/interop/python/unusualDirStructure/use_testing.py diff --git a/test/interop/python/unusualDirStructure/.gitignore b/test/interop/python/unusualDirStructure/.gitignore new file mode 100644 index 000000000000..0a18dfd839dc --- /dev/null +++ b/test/interop/python/unusualDirStructure/.gitignore @@ -0,0 +1 @@ +myLoc/* diff --git a/test/interop/python/unusualDirStructure/COMPOPTS b/test/interop/python/unusualDirStructure/COMPOPTS new file mode 100644 index 000000000000..7d83b267cfa9 --- /dev/null +++ b/test/interop/python/unusualDirStructure/COMPOPTS @@ -0,0 +1 @@ +--library --library-python diff --git a/test/interop/python/unusualDirStructure/PREDIFF b/test/interop/python/unusualDirStructure/PREDIFF new file mode 100755 index 000000000000..bb1950019a2a --- /dev/null +++ b/test/interop/python/unusualDirStructure/PREDIFF @@ -0,0 +1,25 @@ +#!/bin/bash + +# Remove clang/gcc unused argument warning +cat $2 | sed '/^clang: warning: argument unused/d' | \ +sed '/warning: unrecognized command line option/d' | \ +sed '/warning: code will never be executed/d' | \ +# Remove warning about the object file being made for a different OSX version +sed '/^ld: warning: object file/d' | \ +# Remove warning about .a's being built for a different architecture +sed '/ignoring file/d' | \ +# Remove numpy version deprecation warning (Cython is responsible for this) +sed '/arrayobject.h:[0-9]*/d' | \ +sed '/ndarraytypes.h:[0-9]*/d' | \ +sed '/Using deprecated NumPy API/d' | \ +sed '/^ *\^/d' | \ +sed '/^1 warning generated./d' | \ +sed '/^[2-9] warnings generated./d' | \ +sed '/^[ ]*$/d' | \ +sed '/^[ ]*[0-9]* | /d' | \ +sed '/^ *|/d' | \ +# avoid sporadic ld: warning errors (I'm sometimes seeing just that on a line, +# or just multiple copies of it) +sed '/^ld: warning/d' > $2.tmp +cat $2.tmp > $2 +rm $2.tmp diff --git a/test/interop/python/unusualDirStructure/SKIPIF b/test/interop/python/unusualDirStructure/SKIPIF new file mode 100755 index 000000000000..4dc3af94d658 --- /dev/null +++ b/test/interop/python/unusualDirStructure/SKIPIF @@ -0,0 +1,14 @@ +#!/bin/bash + +if [[ $CHPL_COMM != none ]] ; then + echo True +elif [[ $CHPL_TARGET_PLATFORM != darwin && $CHPL_LIB_PIC != pic ]] ; then + echo True +elif command -v python3 >/dev/null; then + # Python3 is required for building with Cython (we would need to change our + # output of certain types and that isn't particularly worth it) + # The script skipif_helper.py checks that cython and numpy are installed + python3 skipif_helper.py +else + echo True +fi diff --git a/test/interop/python/unusualDirStructure/myLoc.notest b/test/interop/python/unusualDirStructure/myLoc.notest new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/test/interop/python/unusualDirStructure/skipif_helper.py b/test/interop/python/unusualDirStructure/skipif_helper.py new file mode 100644 index 000000000000..ab17b05109e1 --- /dev/null +++ b/test/interop/python/unusualDirStructure/skipif_helper.py @@ -0,0 +1,14 @@ +import os + +try: + # numpy and cython are required for building Python modules + import numpy + import cython + + compiler = os.environ['CHPL_TARGET_COMPILER'] + if (compiler.startswith("cray-prgenv") and compiler != "cray-prgenv-gnu"): + print(True) + else: + print(False) +except ImportError: + print(True) diff --git a/test/interop/python/unusualDirStructure/testing.chpl b/test/interop/python/unusualDirStructure/testing.chpl new file mode 100644 index 000000000000..48cdde0488d9 --- /dev/null +++ b/test/interop/python/unusualDirStructure/testing.chpl @@ -0,0 +1,30 @@ +// Chapel file that exports a variety of functions for use with Python +export proc foo() { + writeln("I do nothing"); +} + +export proc takesAndReturns(x: int): int { + writeln("arg was: ", x, ", return is: ", (x + 1)); + return x + 1; +} + +export proc parallelProg(numTasksToRun: int) { + coforall i in 1..numTasksToRun { + writeln("In task ", i); + } +} + +export proc threadring(passes: int, tasks: int) { + var mailbox: [1..tasks] sync int; + mailbox[1].writeEF(0); + proc passTokens(tid) { + do { + const numPasses = mailbox[tid].readFE(); + mailbox[tid%tasks+1].writeEF(numPasses+1); + if numPasses == passes then + writeln(tid); + } while (numPasses < passes); + } + coforall tid in 1..tasks do + passTokens(tid); +} diff --git a/test/interop/python/unusualDirStructure/testing.cleanfiles b/test/interop/python/unusualDirStructure/testing.cleanfiles new file mode 100644 index 000000000000..0a18dfd839dc --- /dev/null +++ b/test/interop/python/unusualDirStructure/testing.cleanfiles @@ -0,0 +1 @@ +myLoc/* diff --git a/test/interop/python/unusualDirStructure/testing.compopts b/test/interop/python/unusualDirStructure/testing.compopts new file mode 100644 index 000000000000..30ef1cb40db7 --- /dev/null +++ b/test/interop/python/unusualDirStructure/testing.compopts @@ -0,0 +1 @@ +--library-dir=myLoc diff --git a/test/interop/python/unusualDirStructure/testing.good b/test/interop/python/unusualDirStructure/testing.good new file mode 100644 index 000000000000..1c36c857e72f --- /dev/null +++ b/test/interop/python/unusualDirStructure/testing.good @@ -0,0 +1,3 @@ +I do nothing +arg was: 3, return is: 4 +21 diff --git a/test/interop/python/unusualDirStructure/testing.noexec b/test/interop/python/unusualDirStructure/testing.noexec new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/test/interop/python/unusualDirStructure/testing.prediff b/test/interop/python/unusualDirStructure/testing.prediff new file mode 100755 index 000000000000..636fb80c4249 --- /dev/null +++ b/test/interop/python/unusualDirStructure/testing.prediff @@ -0,0 +1,9 @@ +#!/bin/bash + +sed '/from testing.c:[0-9]*/d' $2 > $2.tmp +sed '/^ *$/d' $2.tmp > $2 +rm $2.tmp +export PYTHONPATH=myLoc/ +# intentionally include any error output from running the Python file +# (tbh, I should be doing that anyways...) +python3 use_testing.py >> $2 2>&1 diff --git a/test/interop/python/unusualDirStructure/use_testing.py b/test/interop/python/unusualDirStructure/use_testing.py new file mode 100644 index 000000000000..6089a2d30868 --- /dev/null +++ b/test/interop/python/unusualDirStructure/use_testing.py @@ -0,0 +1,9 @@ +# import via the current working directory, since we have an `__init__.py` there +import myLoc.testing as testing + +testing.chpl_setup() +testing.foo() +x = 3 +y = testing.takesAndReturns(x) +testing.threadring(1000, 98) +testing.chpl_cleanup() From 2dbff81db391ea83b30264ddeb4419cc5dc9a6c6 Mon Sep 17 00:00:00 2001 From: Lydia Duncan Date: Tue, 12 Nov 2024 12:54:45 -0800 Subject: [PATCH 2/2] Make chpl_library_finalize re-entrant We were encountering a problem when chpl_library_finalize was getting called multiple times - exiting the Python program would result in: ``` Assertion failed: (data && data->bundle), function chpl_task_getInfoChapel, file chpl-tasks-impl-fns.h, line 140. ``` because our generated `__init__.py` file added an `atexit` call to clean up the runtime, but code could invoke an explicit cleanup call as well. I need to take a look if we explicitly state you need to make that cleanup call, but this may also help in cases where multiple Chapel libraries are used in a Python program. ---- Signed-off-by: Lydia Duncan --- runtime/src/chpl-init.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/runtime/src/chpl-init.c b/runtime/src/chpl-init.c index 41a8a2f65a3e..b04a0f5991b6 100644 --- a/runtime/src/chpl-init.c +++ b/runtime/src/chpl-init.c @@ -441,6 +441,8 @@ void chpl_library_init(int argc, char* argv[]) { // we may have initialized extern void chpl_deinitModules(void); +bool lib_deinited = false; + // // A wrapper around chplexit.c:chpl_finalize(...), sole purpose is // to provide a "chpl_library_*" interface for the Chapel "library-user". @@ -448,6 +450,14 @@ void chpl_library_finalize(void) { if (!lib_inited) { return; } + + if (lib_deinited) { + // Nothing to do, we've already been cleaned up + return; + } else { + lib_deinited = true; + } + chpl_libraryModuleLevelCleanup(); chpl_deinitModules(); chpl_finalize(0, 1);