Merge lp:~afrantzis/mir/fix-1353461-prompt-session-fd-leaks into lp:mir

[Alexandros Frantzis]

frontend,tests: Fix fd leaks in prompt session frontend code and tests

frontend,tests: Fix fd leaks in prompt session frontend code and tests

This MP fixes two leak sources: one in the tests (non-critical) and one in the prompt session frontend code, which is critical since it will eventually cause the server to run out of file descriptors.

[Alan Griffiths (alan-griffiths)]

36 - static std::size_t const arbritary_fd_request_count = 3;

Personally, I'd leave this definition in the fixture (the test reader really shouldn't care what the number is - hence the "arbritary" name).

review: Approve

[Daniel van Vugt (vanvugt)]

I prefer it as it is... with constants defined closer to where they're used. That's most helpful to the reader.

review: Approve

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1842
http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-ci/2395/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-utopic-i386-build/1325
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-utopic-amd64-build/1331
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-utopic-touch/1310
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/917
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-amd64-ci/917/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/918
        deb: http://jenkins.qa.ubuntu.com/job/mir-team-mir-development-branch-utopic-armhf-ci/918/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/247
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-utopic-armhf/247/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/2395
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11461

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-team-mir-development-branch-ci/2395/rebuild

review: Approve (continuous-integration)

=== modified file 'src/server/frontend/session_mediator.cpp'
--- src/server/frontend/session_mediator.cpp	2014-08-07 13:22:53 +0000
+++ src/server/frontend/session_mediator.cpp	2014-08-14 08:24:45 +0000
@@ -42,6 +42,7 @@
 #include "mir/frontend/screencast.h"
 #include "mir/frontend/prompt_session.h"
 #include "mir/scene/prompt_session_creation_parameters.h"
+#include "mir/fd.h"
 
 #include "mir/geometry/rectangles.h"
 #include "client_buffer_tracker.h"
@@ -510,6 +511,7 @@
         {
             auto const fd = connection_context.fd_for_new_client(connect_handler);
             response->add_fd(fd);
+            resource_cache->save_fd(response, mir::Fd{fd});
         }
     }
 

=== modified file 'tests/acceptance-tests/test_prompt_session_client_api.cpp'
--- tests/acceptance-tests/test_prompt_session_client_api.cpp	2014-07-30 15:25:54 +0000
+++ tests/acceptance-tests/test_prompt_session_client_api.cpp	2014-08-14 08:24:45 +0000
@@ -23,6 +23,7 @@
 #include "mir/scene/session.h"
 #include "mir/frontend/session_credentials.h"
 #include "mir/frontend/shell.h"
+#include "mir/fd.h"
 
 #include "mir_test_doubles/stub_session_authorizer.h"
 #include "mir_test_framework/stubbed_server_configuration.h"
@@ -194,10 +195,8 @@
     MOCK_METHOD2(prompt_session_state_change,
         void(MirPromptSession* prompt_provider, MirPromptSessionState state));
 
-    static std::size_t const arbritary_fd_request_count = 3;
     std::mutex mutex;
-    std::size_t actual_fd_count = 0;
-    int actual_fds[arbritary_fd_request_count] = {};
+    std::vector<mir::Fd> actual_fds;
     std::condition_variable cv;
     bool called_back = false;
 
@@ -256,9 +255,11 @@
     auto const self = static_cast<PromptSessionClientAPI*>(context);
 
     std::unique_lock<decltype(self->mutex)> lock(self->mutex);
-    self->actual_fd_count = count;
-
-    std::copy(fds, fds+count, self->actual_fds);
+
+    self->actual_fds.clear();
+    for (size_t i = 0; i != count; ++i)
+        self->actual_fds.push_back(mir::Fd{fds[i]});
+
     self->called_back = true;
     self->cv.notify_one();
 }
@@ -330,11 +331,13 @@
     MirPromptSession* prompt_session = mir_connection_create_prompt_session_sync(
         connection, application_session_pid, null_state_change_callback, this);
 
+    static std::size_t const arbritary_fd_request_count = 3;
+
     mir_prompt_session_new_fds_for_prompt_providers(
         prompt_session, arbritary_fd_request_count, &client_fd_callback, this);
 
     EXPECT_TRUE(wait_for_callback(std::chrono::milliseconds(500)));
-    EXPECT_THAT(actual_fd_count, Eq(arbritary_fd_request_count));
+    EXPECT_THAT(actual_fds.size(), Eq(arbritary_fd_request_count));
 
     mir_prompt_session_release_sync(prompt_session);
 }