Mir

Merge lp://qastaging/~afrantzis/mir/fix-1335741-surface-destruction-after-teardown into lp://qastaging/mir

fix-1335741-surface-destruction-after-teardown
Merge into development-branch

Proposed by Alexandros Frantzis on 2014-06-30

Status:	Rejected
Rejected by:	Alexandros Frantzis on 2014-06-30
Proposed branch:	lp://qastaging/~afrantzis/mir/fix-1335741-surface-destruction-after-teardown
Merge into:	lp://qastaging/mir
Diff against target:	50 lines (+9/-4) 1 file modified tests/acceptance-tests/test_client_surface_events.cpp (+9/-4)
To merge this branch:	bzr merge lp://qastaging/~afrantzis/mir/fix-1335741-surface-destruction-after-teardown
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2014-06-30
Alan Griffiths			Needs Fixing on 2014-06-30
Cemil Azizoglu (community)		2014-06-30	Approve on 2014-06-30
Review via email: mp+224961@code.qastaging.launchpad.net

Commit message

tests: Don't keep surfaces alive and try to destroy them after the server has been torn down

The server is torn down during TearDown() but we keep a surface alive after that and try to destroy it when destroying the test fixture, leading to occasional crashes.

Description of the change

tests: Don't keep surfaces alive and try to destroy them after the server has been torn down

The server is torn down during TearDown() but we keep a surface alive after that and try to destroy it when destroying the test fixture, leading to occasional crashes.

It is debatable whether we should handle this case more gracefully in the server itself. For further investigation here is the address sanitizer backtrace of the problem:

http://paste.ubuntu.com/7725250/

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2014-06-30:

Looks good (for now).

review: Approve

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-06-30:

I have difficulty reproducing. (I see intermittent "Invalid write" errors with and without this fix but not lp:1335741.)

I think it is simpler to add "scene_surface.reset();" in ClientSurfaceEvents::TearDown() - before the server is torn down in "BasicClientServerFixture::TearDown();".

Does that work for those that can reproduce?

review: Needs Information

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2014-06-30:

I don't think this fix is related to lp:1335741. I also think it can be simplified as above.

lp:1335741 reports an error during the test, not during teardown and can be "fixed" by adding the following line to the start of client_can_query_current_orientation:

EXPECT_FALSE(wait_for_event(mir_event_type_orientation, std::chrono::milliseconds(1)));

That isn't a *good* fix but it does strongly suggests that the problem there is with setup not teardown.

So, I suggest we should break the association with lp:1335741 and then land this (or something equivalent).

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-06-30:

FAILED: Continuous integration, rev:1732
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2027/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/753
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/759
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/752/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/548/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/546
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/546/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2270
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/2270/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/1914/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/9021