Merge lp:~vanvugt/mir/fix-1391976 into lp:mir

[Daniel van Vugt]

Fix^H^H^H Work around protobuf reload issues so that it's always safe
to un/reload libmirprotobuf multiple times. (LP: #1391976)

Technically a proper fix will come from libprotobuf itself but this way
we don't need to wait for that.

I expect CI might reject this reporting leaks. I'll get to that and add any suppressions required tomorrow.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2703
http://jenkins.qa.ubuntu.com/job/mir-ci/4170/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2956
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/471
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2904
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/326
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/326/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2904
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2904/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5728
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21393

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4170/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

OK as a workaround, although I feel slightly uncomfortable since it's not expected for libraries to forcibly keep other libraries in memory.

I am working on the proper fix on the protobuf side, which is to ensure libprotobuf doesn't export unnecessary symbols.

review: Approve

[Kevin DuBois (kdub)]

Alright (and can't think of a better way to do this, short of going upstream)

review: Approve

[Daniel van Vugt (vanvugt)]

It's actually logically perfectly safe. Because already, no function can ever assume there's not some other function in the same process that has loaded and is using the same libraries as itself.

The requirement that modules share the same process and that you can never know the internal reference count of a dl-opened library is pre-existing.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2704
http://jenkins.qa.ubuntu.com/job/mir-ci/4183/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2971
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/486
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2919/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/339
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/339/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2919
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2919/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5741/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21430

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4183/rebuild

review: Needs Fixing (continuous-integration)

[Alberto Aguirre (albaguirre)]

Un TA'ed as pre-req failed autolanding

[Robert Carr (robertcarr)]

Sad but reasonable

review: Approve

[lp:~vanvugt/mir/fix-1391976]

2705. By Daniel van Vugt on 2015-06-25

Merge latest parent branch (and trunk)

[Daniel van Vugt (vanvugt)]

More sad that this is not the first time I've used this workaround. I've had to do it in previous jobs too and on other Unixes.

The CI failures appear to be unrelated infrastructure faults. The first Jenkins run passed...

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2705
http://jenkins.qa.ubuntu.com/job/mir-ci/4195/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2987
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/504
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2935
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/351
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/351/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2935
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2935/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5755
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21467

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4195/rebuild

review: Approve (continuous-integration)

=== modified file 'src/protobuf/CMakeLists.txt'
--- src/protobuf/CMakeLists.txt	2015-06-17 05:20:42 +0000
+++ src/protobuf/CMakeLists.txt	2015-06-25 03:55:46 +0000
@@ -21,7 +21,7 @@
 
 target_link_libraries(
   mirprotobuf
-
+  dl
   ${PROTOBUF_LIBRARIES}
 )
 

=== modified file 'src/protobuf/google_protobuf_guard.cpp'
--- src/protobuf/google_protobuf_guard.cpp	2013-04-24 05:22:20 +0000
+++ src/protobuf/google_protobuf_guard.cpp	2015-06-25 03:55:46 +0000
@@ -20,6 +20,7 @@
 
 #include <google/protobuf/descriptor.h>
 #include <mutex>
+#include <dlfcn.h>
 
 namespace mir
 {
@@ -30,6 +31,14 @@
 
 void init_google_protobuf()
 {
+    // Leak libmirprotobuf.so.X
+    // This will stop it getting unloaded/reloaded and work around LP: #1391976
+    Dl_info self;
+    if (dladdr(reinterpret_cast<void*>(&init_google_protobuf), &self))
+    {
+        dlopen(self.dli_fname, RTLD_LAZY | RTLD_NODELETE);
+    }
+
     GOOGLE_PROTOBUF_VERIFY_VERSION;
 }
 

=== modified file 'tests/loader-tests/CMakeLists.txt'
--- tests/loader-tests/CMakeLists.txt	2015-06-25 03:55:46 +0000
+++ tests/loader-tests/CMakeLists.txt	2015-06-25 03:55:46 +0000
@@ -28,7 +28,6 @@
 
 target_link_libraries(mir_test_reload_protobuf dl)
 
-# FIXME: Enable this test after LP: #1391976 is fixed.
-#add_test(NAME Protobuf-can-be-reloaded   # LP: #1391976
-#  COMMAND ${CMAKE_BINARY_DIR}/bin/mir_test_reload_protobuf
-#)
+add_test(NAME Protobuf-can-be-reloaded   # LP: #1391976
+  COMMAND ${CMAKE_BINARY_DIR}/bin/mir_test_reload_protobuf
+)