sdjournal: tie libsystemdFunctions to Journal's life #169
Conversation
There was a map from function names to their addresses so we don't need to call dlsym everytime we call a function from libsystemd if it was already called before. Every time we open a new Journal, we call dlopen() (and every time we close it, we call dlclose()). Since the map was global and we weren't clearing it up after closing Journal, we were reusing the addresses from the previous invocation. This is wrong because libsystemd can end up in a different address the second time we dlopen() it. Store the map in the Journal struct so it's a per-Journal cache.
iaguis
force-pushed
the
iaguis/libsystemd-map
branch
from
03207cb
to
7094f1d
Compare
iaguis
changed the title
|
LGTM. |
|
LGTM if Semaphore is green. As said OOB, it's probably better to keep the library open rather than doing dlopen/dlclose at each method call. But this PR is a small patch that we can easily get in for the next release. |
FWIW we only do it when the user creates a new Journal (and when she closes it). |
|
I'm not sure this is a great approach: memory footprint will now be linear in the number of journal instances, and every instance will go through several misses every time (to populate symbols cache). Book-keeping overhead, in particular on short lived instances, seems considerable now. Can't we just dlopen it once and then let it go away on process exit, reusing the same symbols cache for all instances? |
|
This PR implements the suggestion on #169 (comment): #170 |
|
Superseded by #170. |
There was a map from function names to their addresses so we don't
need to call dlsym everytime we call a function from libsystemd if it
was already called before.
Every time we open a new Journal, we call dlopen() (and every time we
close it, we call dlclose()). Since the map was global and we weren't
clearing it up after closing Journal, we were reusing the addresses from
the previous invocation. This is wrong because libsystemd can end up in
a different address the second time we dlopen() it.
Store the map in the Journal struct so it's a per-Journal cache.
Should fix rkt/rkt#2740