Mir

Merge lp:~albaguirre/mir/fix-1573572 into lp:mir

  1. fix-1573572
  2. Merge into development-branch
Proposed by Alberto Aguirre
Status: Merged
Merged at revision: 3474
Proposed branch: lp:~albaguirre/mir/fix-1573572
Merge into: lp:mir
Diff against target: 170 lines (+90/-0)
5 files modified
src/server/frontend/screencast_buffer_tracker.cpp (+8/-0)
src/server/frontend/screencast_buffer_tracker.h (+5/-0)
src/server/frontend/session_mediator.cpp (+15/-0)
src/server/frontend/session_mediator.h (+2/-0)
tests/unit-tests/frontend/test_session_mediator.cpp (+60/-0)
To merge this branch: bzr merge lp:~albaguirre/mir/fix-1573572
Reviewer Review Type Date Requested Status
Mir CI Bot continuous-integration Needs Fixing
Cemil Azizoglu (community) Approve
Review via email: mp+292653@code.launchpad.net

Commit message

Remove orphaned screencast sessions on disconnection

When a client disconnects either normally or due to a crash, remove any screencasts associated with the session.

To post a comment you must log in.
Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

Looks good

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

PASSED: Continuous integration, rev:3473
https://mir-jenkins.ubuntu.com/job/mir-ci/892/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/926
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/965
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/956
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/956
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/936
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/936/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/936
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/936/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/936
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/936/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/936
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/936/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/936
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/936/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
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/221/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/927/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/237/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/966
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/957
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/957
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/937/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/937
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/937/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/937
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/937/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/937
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/937/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/937
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/937/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

"[FAILED] ClientLatency.triple_buffered_client_has_less_than_two_frames_latency"

Unrelated.

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/222/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/929/console
    None: https://mir-jenkins.ubuntu.com/job/generic-land-mp/238/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/968
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/959
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/959
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/939/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/939/artifact/output/*zip*/output.zip
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/939/console
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/939/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/939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/939/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/939
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/939/artifact/output/*zip*/output.zip

review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Aguirre (albaguirre) wrote :

"21:49:08 + echo 'SETUP FAILED - CAN'\''T CONTINUE'
21:49:08 SETUP FAILED - CAN'T CONTINUE"

Looks like a device setup issue.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/server/frontend/screencast_buffer_tracker.cpp'
--- src/server/frontend/screencast_buffer_tracker.cpp 2016-04-21 00:30:17 +0000
+++ src/server/frontend/screencast_buffer_tracker.cpp 2016-04-22 15:21:32 +0000
@@ -31,3 +31,11 @@
31{31{
32 tracker.erase(id);32 tracker.erase(id);
33}33}
34
35void mf::ScreencastBufferTracker::for_each_session(std::function<void(ScreencastSessionId)> f) const
36{
37 for (auto const& entry : tracker)
38 {
39 f(entry.first);
40 }
41}
3442
=== modified file 'src/server/frontend/screencast_buffer_tracker.h'
--- src/server/frontend/screencast_buffer_tracker.h 2016-04-21 00:30:17 +0000
+++ src/server/frontend/screencast_buffer_tracker.h 2016-04-22 15:21:32 +0000
@@ -20,6 +20,7 @@
2020
21#include "mir/frontend/screencast.h"21#include "mir/frontend/screencast.h"
2222
23#include <functional>
23#include <unordered_set>24#include <unordered_set>
24#include <unordered_map>25#include <unordered_map>
2526
@@ -49,6 +50,10 @@
49 /* removes all tracked buffers associated with id */50 /* removes all tracked buffers associated with id */
50 void remove_session(ScreencastSessionId id);51 void remove_session(ScreencastSessionId id);
5152
53 /* Iterates over all tracked session ids */
54 void for_each_session(std::function<void(ScreencastSessionId)> f) const;
55
56
52private:57private:
53 ScreencastBufferTracker(ScreencastBufferTracker const&) = delete;58 ScreencastBufferTracker(ScreencastBufferTracker const&) = delete;
54 ScreencastBufferTracker& operator=(ScreencastBufferTracker const&) = delete;59 ScreencastBufferTracker& operator=(ScreencastBufferTracker const&) = delete;
5560
=== modified file 'src/server/frontend/session_mediator.cpp'
--- src/server/frontend/session_mediator.cpp 2016-04-21 00:30:17 +0000
+++ src/server/frontend/session_mediator.cpp 2016-04-22 15:21:32 +0000
@@ -121,6 +121,7 @@
121 report->session_error(session->name(), __PRETTY_FUNCTION__, "connection dropped without disconnect");121 report->session_error(session->name(), __PRETTY_FUNCTION__, "connection dropped without disconnect");
122 shell->close_session(session);122 shell->close_session(session);
123 }123 }
124 destroy_screencast_sessions();
124}125}
125126
126void mf::SessionMediator::client_pid(int pid)127void mf::SessionMediator::client_pid(int pid)
@@ -536,6 +537,7 @@
536 report->session_disconnect_called(session->name());537 report->session_disconnect_called(session->name());
537538
538 shell->close_session(session);539 shell->close_session(session);
540 destroy_screencast_sessions();
539 weak_session.reset();541 weak_session.reset();
540542
541 done->Run();543 done->Run();
@@ -1173,3 +1175,16 @@
11731175
1174 return config;1176 return config;
1175}1177}
1178
1179void mf::SessionMediator::destroy_screencast_sessions()
1180{
1181 std::vector<ScreencastSessionId> ids_to_untrack;
1182 screencast_buffer_tracker.for_each_session([this, &ids_to_untrack](ScreencastSessionId id)
1183 {
1184 screencast->destroy_session(id);
1185 ids_to_untrack.push_back(id);
1186 });
1187
1188 for (auto const& id : ids_to_untrack)
1189 screencast_buffer_tracker.remove_session(id);
1190}
11761191
=== modified file 'src/server/frontend/session_mediator.h'
--- src/server/frontend/session_mediator.h 2016-04-21 00:30:17 +0000
+++ src/server/frontend/session_mediator.h 2016-04-22 15:21:32 +0000
@@ -249,6 +249,8 @@
249249
250 virtual std::function<void(std::shared_ptr<Session> const&)> prompt_session_connect_handler() const;250 virtual std::function<void(std::shared_ptr<Session> const&)> prompt_session_connect_handler() const;
251251
252 void destroy_screencast_sessions();
253
252 pid_t client_pid_;254 pid_t client_pid_;
253 std::shared_ptr<Shell> const shell;255 std::shared_ptr<Shell> const shell;
254 std::shared_ptr<graphics::PlatformIpcOperations> const ipc_operations;256 std::shared_ptr<graphics::PlatformIpcOperations> const ipc_operations;
255257
=== modified file 'tests/unit-tests/frontend/test_session_mediator.cpp'
--- tests/unit-tests/frontend/test_session_mediator.cpp 2016-04-21 00:30:17 +0000
+++ tests/unit-tests/frontend/test_session_mediator.cpp 2016-04-22 15:21:32 +0000
@@ -44,6 +44,7 @@
44#include "mir/test/doubles/mock_frontend_surface.h"44#include "mir/test/doubles/mock_frontend_surface.h"
45#include "mir/test/doubles/mock_event_sink.h"45#include "mir/test/doubles/mock_event_sink.h"
46#include "mir/test/doubles/mock_event_sink_factory.h"46#include "mir/test/doubles/mock_event_sink_factory.h"
47#include "mir/test/doubles/mock_screencast.h"
47#include "mir/test/doubles/stub_buffer.h"48#include "mir/test/doubles/stub_buffer.h"
48#include "mir/test/doubles/mock_buffer.h"49#include "mir/test/doubles/mock_buffer.h"
49#include "mir/test/doubles/stub_session.h"50#include "mir/test/doubles/stub_session.h"
@@ -266,6 +267,20 @@
266 mt::fake_shared(mock_hub));267 mt::fake_shared(mock_hub));
267 }268 }
268269
270 std::shared_ptr<mf::SessionMediator> create_session_mediator_with_screencast(
271 std::shared_ptr<mf::Screencast> const& screencast)
272 {
273 return std::make_shared<mf::SessionMediator>(
274 shell, mt::fake_shared(mock_ipc_operations), graphics_changer,
275 surface_pixel_formats, report,
276 std::make_shared<mtd::NullEventSinkFactory>(),
277 std::make_shared<mtd::NullMessageSender>(),
278 resource_cache, screencast, &connector, nullptr, nullptr,
279 std::make_shared<mtd::NullANRDetector>(),
280 mir::cookie::Authority::create(),
281 mt::fake_shared(mock_hub));
282 }
283
269 MockConnector connector;284 MockConnector connector;
270 testing::NiceMock<mtd::MockPlatformIpcOperations> mock_ipc_operations;285 testing::NiceMock<mtd::MockPlatformIpcOperations> mock_ipc_operations;
271 testing::NiceMock<mtd::MockInputDeviceHub> mock_hub;286 testing::NiceMock<mtd::MockInputDeviceHub> mock_hub;
@@ -1321,3 +1336,48 @@
13211336
1322 EXPECT_THAT(connection.input_devices(), mt::InputDevicesMatch(devices));1337 EXPECT_THAT(connection.input_devices(), mt::InputDevicesMatch(devices));
1323}1338}
1339
1340TEST_F(SessionMediator, disconnect_removes_orphaned_screencast_sessions)
1341{
1342 using namespace ::testing;
1343 auto mock_screencast = std::make_shared<NiceMock<mtd::MockScreencast>>();
1344 auto mediator = create_session_mediator_with_screencast(mock_screencast);
1345
1346 mf::ScreencastSessionId expected_id{7};
1347 mtd::StubBuffer stub_buffer;
1348 ON_CALL(*mock_screencast, create_session(_,_,_,_,_))
1349 .WillByDefault(Return(expected_id));
1350 ON_CALL(*mock_screencast, capture(_))
1351 .WillByDefault(Return(mt::fake_shared(stub_buffer)));
1352
1353 EXPECT_CALL(*mock_screencast, destroy_session(expected_id));
1354
1355 mp::ScreencastParameters screencast_parameters;
1356 mp::Screencast screencast;
1357 mediator->connect(&connect_parameters, &connection, null_callback.get());
1358 mediator->create_screencast(&screencast_parameters, &screencast, null_callback.get());
1359 mediator->disconnect(nullptr, nullptr, null_callback.get());
1360}
1361
1362TEST_F(SessionMediator, destructor_removes_orphaned_screencast_sessions)
1363{
1364 using namespace ::testing;
1365 auto mock_screencast = std::make_shared<NiceMock<mtd::MockScreencast>>();
1366 auto mediator = create_session_mediator_with_screencast(mock_screencast);
1367
1368 mf::ScreencastSessionId expected_id{7};
1369 mtd::StubBuffer stub_buffer;
1370 ON_CALL(*mock_screencast, create_session(_,_,_,_,_))
1371 .WillByDefault(Return(expected_id));
1372 ON_CALL(*mock_screencast, capture(_))
1373 .WillByDefault(Return(mt::fake_shared(stub_buffer)));
1374
1375 EXPECT_CALL(*mock_screencast, destroy_session(expected_id));
1376
1377 mp::ScreencastParameters screencast_parameters;
1378 mp::Screencast screencast;
1379 mediator->create_screencast(&screencast_parameters, &screencast, null_callback.get());
1380
1381 mediator.reset();
1382
1383}

Subscribers

People subscribed via source and target branches