On Tue, May 08, 2012 at 10:34:06AM +0200, Gergely Nagy wrote:
Another thing is that the first patch stops libsyslog-ng-crypto from being directly linked to ivykis. This is not an issue when ivykis is statically compiled into libsyslog-ng, since all those symbols will be exported to other modules, including libsyslog-ng-crypto. However, it might become a problem when using a dynamically linked ivykis. I haven't tested that case yet.
The reason I remove the link is that if ivykis is statically compiled in to both shared objects, the constructors will run twice, as dlopen()ing anything linked against libsyslog-ng-crypto will trigger them again. This leads to an abort() inside ivykis.
This can be done on the ivykis side too: separate the constructors from the init functions, and make the constructors no-op if they ran already: this allows the init functions to continue aborting if called more than once, but allows the constructors to run twice. I have to discuss this with upstream, and see if we need this at all, or if my workaround of removing the linkage from libsyslog-ng-crypto is enough and without side effects.
If the constructors run twice but bss variables are still shared, then yeah, we should do something like what you said.
As far as I see, what happens is that when syslog-ng is started, the ivykis statically linked into libsyslog-ng gets loaded and the constructors run. So far, so good, that's how it is supposed to work!
When syslog-ng dlopen()s a module linked against libsyslog-ng-crypto, which also has an ivykis statically linked in, ld.so notices that the symbols are already resolved, so won't load them again, BUT the constructors *are* different, since the constructor can be thought of as a function in the lib that runs whatever we had an __attribute__((constructor))__ on.
This means that the init functions will run again, even though they were not redefined, because the newly loaded constructor-collecting function-like thingy does get run regardless.
This means that constructors and destructors should be made idempotent. Which might be tricky if we consider that a module can (in theory, syslog-ng doesn't do this AFAIK) be unloaded, which would trigger the destructor, while the main code would still use it.
So deinit should only be called from the destructors if there are no users left.
This makes things a little bit more complicated, I think. For syslog-ng, it's enough if nothing but libsyslog-ng gets linked to ivykis, so my workaround will do the trick. For other things, though, this may not be enough.
I thought about this for a while, but couldn't really figure out how this should work under the hood. So I tried it out in practice, and I indeed can't seem to make it work in a way that makes sense. I whipped up the following test setup: tst.c statically linking against libtest.a (built from libtest.c), and dynamically loading libplugin.so (built from libplugin.c) which is also statically linked against libtest.a. First of all, I need to build libtest.a with -fPIC, or gcc won't let me link the non-PIC code in libtest.a into libplugin.so (which only makes sense, I guess). Do you need to do that too, or am I testing something different from your setup at this point? Having done that, I indeed see a constructor function in libtest.c being called both before main() and when libplugin.so is dlopen()ed, i.e. it's called twice like what you report above. But then, if I define a static variable in libtest.c, and print its address in that constructor function, I get different addresses for each of the invocations of the constructor. I.e. the libtest.a linked into tst.c is using a different set of static variables than the libtest.a linked into libplugin.so. If the same happens in your setup, I don't see how we can fix this except building libivykis as a .so instead of as a .a -- but if you're not seeing this, I'd love to know what I'm doing wrong... (If I make the global variable in libtest.c non-static, both invocations of the libtest constructor see the same address for the variable. But, most global ivykis variables are static..)