terralang / terra

Terra is a low-level system programming language that is embedded in and meta-programmed by the Lua programming language.
terralang.org
Other
2.71k stars 197 forks source link

Run LLVM shutdown via atexit when possible #587

Closed elliottslaughter closed 2 years ago

elliottslaughter commented 2 years ago

For context, see this thread: https://github.com/StanfordLegion/legion/issues/1297#issuecomment-1188635075

It seems that it is unsafe to call atexit inside a shared library loaded with terralib.linklibrary. For various reasons, turning this off in all possible libraries may not be feasible or reasonable. Instead, this PR attempts to make this safe by calling terra_llvmshutdown from atexit (falling back to just calling it at the end of main otherwise). While atexit can lead to some screwy behavior, this seems like the least-bad solution, in that it makes it possible to (I think safely) use atexit in libraries loaded by terralib.linklibrary, making this one less thing users need to think about (hopefully).

Why is this safe?

We call atexit(terra_llvmshutdown) before executing or initializing any Terra code. atexit handlers run in LIFO order. Therefore, any atexit handlers registered by libraries loaded via terralib.linklibrary will have an opportunity to run before terra_llvmshutdown attempts to unload them.

The main thing I'm dissatisfied about in this change is that this also influences the explicit exit(1) code path. In the past, we did not attempt to register anything to run in these cases, and arguably, it's not even worth attempting to shut down LLVM on a non-clean exit. However, I'm not aware of a way to say "register this atexit handler only for the normal exit case", so I'm not sure what else to do about this.

elliottslaughter commented 2 years ago

We changed the application to use dlopen, which does not automatically close and therefore does not interfere with atexit. It's a bit of a pain, but I think it's the best solution because otherwise this is a change in behavior that isn't really desirable (and in a way that can't fully be mitigated; i.e. I'm not aware of a way to install an atexit handler on the normal exit path only).