[WIP]Fix double dlclose when uninstalling bundle without activator #301
Conversation
For bundle without activator, all handles of its shared objects are immediately closed after dlopen. Thus they should NOT be added to a revision's handle-list.
Codecov Report
@@ Coverage Diff @@
## master #301 +/- ##
=======================================
Coverage 66.74% 66.74%
=======================================
Files 147 147
Lines 29947 29947
=======================================
Hits 19987 19987
Misses 9960 9960
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
First off, I am not very familiar with this part of Celix (so loading/closing libraries).
I did some improvement for this, but this was very "just fix it" oriented.
I also think that loadLibraries/closeLibraries handling could use a refactoring to make this more cleaner, less code copies and in a separate source / header file from the framework.c.
Note that framework.c is more that 2500 lines of code and that makes it very hard to maintain.
Looking a the stacktrace I see that it tries to close the library at
celix/libs/framework/src/framework.c
Line 2258 in 40e33da
I think the bug is in the
else if condition.
The if will set the activatorHandle to the current handle if that is the activator library and if this was loaded successful. Ok nothing wrong there.
The else if will just close the library if the handle is not null, meaning all libraries loaded that are not the activator will be closed here. This is not what I expected.
I expect the whole handeling of a loadLibrary result to be something like:
if (status == CELIX_SUCCESS) {
//ok, library loaded successful
if (activator != NULL && strcmp(trimmedLib, activator) == 0) {
//the activator library was loaded, setting activatorHandle
*activatorHandle = handle;
}
} else {
//TODO log error
assert(handle == NULL); //framework_loadLibrary should not return a handle if it is not successful
}| @@ -2307,16 +2317,6 @@ static celix_status_t framework_loadLibrary(framework_pt framework, const char * | |||
| if (*handle == NULL) { | |||
| error = celix_libloader_getLastError(); | |||
| status = CELIX_BUNDLE_EXCEPTION; | |||
| } else { | |||
There was a problem hiding this comment.
Removing this and moving this to loadLibraries (and only for the activator libraries) means that not all libraries mentioned in Private-Library and Import-Library are added to the handles list.
I think this is not the desired result.
I expected this issues to be solved during the stopping if a bundle (i.e. during the closing of a library).
Could you first add an unit test to reproduce this issue. We already have some test for starting/stopping bundles. See
celix/libs/framework/gtest/CMakeLists.txt
Line 18 in 40e33da
PengZheng
changed the title
|
It should have been fixed by #476 |
I kept playing with the export_import example, and found that the handle to libhello_test2libd.so.3 has been closed twice, but opened only once.