Mir

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

Proposed by Brandon Schaefer on 2017-03-07
Status: Rejected
Rejected by: Brandon Schaefer on 2017-03-17
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 2017-03-07 Approve on 2017-03-16
Mir CI Bot continuous-integration Needs Fixing on 2017-03-14
Kevin DuBois (community) Approve on 2017-03-13
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.
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)
Daniel van Vugt (vanvugt) wrote :

Nice.

I have verified bug 1667352 is fixed with this.

review: Approve
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)
Daniel van Vugt (vanvugt) wrote :

^^^
Protobuf issues and others

Daniel van Vugt (vanvugt) wrote :

^^^
Bug 1671370 is one of the issues

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.

Kevin DuBois (kdub) wrote :

alright

review: Approve
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)
Brandon Schaefer (brandontschaefer) wrote :

Gonna run one more time just to be sure

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)
Brandon Schaefer (brandontschaefer) wrote :

Seems happy!

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)
Daniel van Vugt (vanvugt) wrote :

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

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?

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?

Daniel van Vugt (vanvugt) :
review: Needs Information
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!

Brandon Schaefer (brandontschaefer) wrote :

nm its failing on zesty just not locally!

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...

Daniel van Vugt (vanvugt) :
review: Approve
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