Mir

Merge lp:~brandontschaefer/mir/mirclient-reload into lp:mir

Proposed by Brandon Schaefer
Status: Rejected
Rejected by: Brandon Schaefer
Proposed branch: lp:~brandontschaefer/mir/mirclient-reload
Merge into: lp:mir
Diff against target: 17 lines (+3/-0)
1 file modified
src/protobuf/google_protobuf_guard.cpp (+3/-0)
To merge this branch: bzr merge lp:~brandontschaefer/mir/mirclient-reload
Reviewer Review Type Date Requested Status
Daniel van Vugt Approve
Mir CI Bot continuous-integration Needs Fixing
Kevin DuBois (community) Approve
Review via email: mp+319262@code.launchpad.net

Commit message

Workaround for the issue of libprotobuf-lite being leaked after a dlclose to libmirclient. As it gets leaked, its static variables dont get reset to default values. Meaning their empty string optimization wont be recreated leaving a dangling pointer, pointing to the memory that was created in the first dlopen.
(LP: #1670844, LP: #1667352, LP: #1667542)

Description of the change

Workaround for the issue of libprotobuf-lite being leaked after a dlclose to libmirclient. As it gets leaked, its static variables dont get reset to default values. Meaning their empty string optimization wont be recreated leaving a dangling pointer, pointing to the memory that was created in the first dlopen.

Looking into a test, but its a bit difficult. As you've to:
dlopen libmirclient
load a libmirclient symbol that happens to call a protobuf function which will end up using a string
check that it segfaults/yells when that function is called

To post a comment you must log in.
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4061
https://mir-jenkins.ubuntu.com/job/mir-ci/3108/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4170
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4257
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4247
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4247
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4247
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4197
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4197/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4197
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4197/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4197
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4197/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4197
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4197/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4197
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4197/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4197
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4197/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3108/rebuild

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Nice.

I have verified bug 1667352 is fixed with this.

review: Approve
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/1179/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/4184/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/1243/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4271
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4261
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4261
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4261
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4211/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4211/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4211/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4211/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4211/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4211/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4211/console

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

^^^
Protobuf issues and others

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

^^^
Bug 1671370 is one of the issues

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Going to wait for the capnproto fix to land. Though need to check that strange valgrind yelling. Didnt see that on my end.

Revision history for this message
Kevin DuBois (kdub) wrote :

alright

review: Approve
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4061
https://mir-jenkins.ubuntu.com/job/mir-ci/3144/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4218
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4305
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4295
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4295
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4295
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4245
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4245/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4245
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4245/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4245
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4245/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4245
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4245/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4245
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4245/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4245
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4245/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3144/rebuild

review: Approve (continuous-integration)
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Gonna run one more time just to be sure

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4061
https://mir-jenkins.ubuntu.com/job/mir-ci/3146/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4220
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4307
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4297
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4297
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4297
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4247/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4247/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4247/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4247/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4247/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4247
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4247/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3146/rebuild

review: Approve (continuous-integration)
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Seems happy!

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

FAILED: Autolanding.
More details in the following jenkins job:
https://mir-jenkins.ubuntu.com/job/mir-autolanding/1187/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/4222/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/1252/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4309
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/4299
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/4299
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4299
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4249
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4249/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/4249/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4249/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4249/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/4249/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/4249/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/4249/console

review: Needs Fixing (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Nope. Autolanding seems to have a slightly different test environment.

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Yeah it does... not sure whats its doing, as with sbuild on x + o im not seeing this leak. The leak happens in generated code (or at lease the top level of yelling) which doesnt match up with my generation. Going to attempt figure this out a bit more though could be an issue with a protobuf version?

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Last I heard we were waiting on alf to re-upstream the fix for protobuf. So when that's done, do we need this branch at all?

Revision history for this message
Daniel van Vugt (vanvugt) :
review: Needs Information
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Yes this is true. Though his main upstream will be protobuf(13?) which zesty uses 10 and xenial uses 9v5. As looking at this CI failure it seems to happen only in x + o and v + o. So something to do with the older protobuf versions.

Im happy with letting alf get his versions upstreamed then we can ubuntu patch protobuf 9v5 or backport 10 to the overlay.

Ill still try to get a workaround in place, seems to be a strange invalid read then delete on a pointer that is first init'ed with the library. Need to figure out a way to reproduce!

Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

nm its failing on zesty just not locally!

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Well, this is only really to fix SDL games right now so if we only get a fix into 17.10 that might be OK...

Revision history for this message
Daniel van Vugt (vanvugt) :
review: Approve
Revision history for this message
Brandon Schaefer (brandontschaefer) wrote :

Alf already has this fix upstream and a ppu up for zesty soo lets not merge this

Unmerged revisions

4061. By Brandon Schaefer on 2017-03-07

* A workaround for re-loading libmirclient multiple times.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/protobuf/google_protobuf_guard.cpp'
2--- src/protobuf/google_protobuf_guard.cpp 2016-01-29 08:18:22 +0000
3+++ src/protobuf/google_protobuf_guard.cpp 2017-03-07 22:25:29 +0000
4@@ -17,11 +17,14 @@
5 */
6
7 #include <google/protobuf/stubs/common.h>
8+#include <google/protobuf/generated_message_util.h>
9
10 extern "C" int __attribute__((constructor))
11 init_google_protobuf()
12 {
13 GOOGLE_PROTOBUF_VERIFY_VERSION;
14+ // Work around for bug LP: #1670844
15+ ::google::protobuf::internal::empty_string_once_init_ = false;
16 return 0;
17 }
18

Subscribers

People subscribed via source and target branches