Skip to content

Commit

Permalink
Avoid a double free with generated Python libraries (#26228)
Browse files Browse the repository at this point in the history
[reviewed by @dlongnecke-cray]

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.
This would cause problems if the generated library was imported via the
local directory structure instead of the PYTHONPATH, as the
`__init__.py` would apply to the outer directory being referenced and
thus would get registered as part of the `import` to get the inner
library. Removing the explicit clean up in user code would help, but we
shouldn't be sensitive to how the user got access to our exported
functions, nor if they feel like making the explicit cleanup call
themselves.

This may also help in cases where multiple Chapel libraries are used in
a Python program.

Add a test locking in the behavior I was seeing with an unusual import
path and that it has been fixed

Passed a local run of all the Python single locale interop tests, and a
normal paratest with a fresh checkout (to make sure I didn't break C
interoperability)
  • Loading branch information
lydia-duncan authored Nov 13, 2024
2 parents 38241cb + 2dbff81 commit ed9522e
Show file tree
Hide file tree
Showing 14 changed files with 118 additions and 0 deletions.
10 changes: 10 additions & 0 deletions runtime/src/chpl-init.c
Original file line number Diff line number Diff line change
Expand Up @@ -441,13 +441,23 @@ 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".
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);
Expand Down
1 change: 1 addition & 0 deletions test/interop/python/unusualDirStructure/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
myLoc/*
1 change: 1 addition & 0 deletions test/interop/python/unusualDirStructure/COMPOPTS
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--library --library-python
25 changes: 25 additions & 0 deletions test/interop/python/unusualDirStructure/PREDIFF
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions test/interop/python/unusualDirStructure/SKIPIF
Original file line number Diff line number Diff line change
@@ -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
Empty file.
14 changes: 14 additions & 0 deletions test/interop/python/unusualDirStructure/skipif_helper.py
Original file line number Diff line number Diff line change
@@ -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)
30 changes: 30 additions & 0 deletions test/interop/python/unusualDirStructure/testing.chpl
Original file line number Diff line number Diff line change
@@ -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);
}
1 change: 1 addition & 0 deletions test/interop/python/unusualDirStructure/testing.cleanfiles
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
myLoc/*
1 change: 1 addition & 0 deletions test/interop/python/unusualDirStructure/testing.compopts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
--library-dir=myLoc
3 changes: 3 additions & 0 deletions test/interop/python/unusualDirStructure/testing.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
I do nothing
arg was: 3, return is: 4
21
Empty file.
9 changes: 9 additions & 0 deletions test/interop/python/unusualDirStructure/testing.prediff
Original file line number Diff line number Diff line change
@@ -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
9 changes: 9 additions & 0 deletions test/interop/python/unusualDirStructure/use_testing.py
Original file line number Diff line number Diff line change
@@ -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()

0 comments on commit ed9522e

Please sign in to comment.