Merge lp:~kdub/mir/fix-1192742 into lp:~mir-team/mir/trunk

[Kevin DuBois]

fix: lp1192742 by cleaning up FD's when an android client unregisters its native_handle_t

fix: lp1192742 by cleaning up FD's when an android client unregisters its native_handle_t

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:804
http://jenkins.qa.ubuntu.com/job/mir-ci/884/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1129
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1014
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/122
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/122/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/535

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/884/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

Looks good.

48 + mcla::AndroidRegistrarGralloc registrar(mock_module);

This calls for a Stub/NullModule, since no expectations are checked, but I won't block.

review: Approve

[Alan Griffiths (alan-griffiths)]

> Looks good.
>
> 48 + mcla::AndroidRegistrarGralloc registrar(mock_module);
>
> This calls for a Stub/NullModule, since no expectations are checked, but I
> won't block.

+1

review: Abstain

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:805
http://jenkins.qa.ubuntu.com/job/mir-ci/894/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/1141
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1026
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/132
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/132/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/542

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/894/rebuild

review: Approve (continuous-integration)

=== modified file 'src/client/android/android_registrar.cpp'
--- src/client/android/android_registrar.cpp	2013-06-21 23:35:50 +0000
+++ src/client/android/android_registrar.cpp	2013-07-03 17:18:27 +0000
@@ -37,6 +37,8 @@
     void operator()(const native_handle_t* t)
     {
         module->unregisterBuffer(module.get(), t);
+        for(auto i=0; i < t->numFds; i++)
+            close(t->data[i]);
         ::operator delete(const_cast<native_handle_t*>(t));
     }
 private:

=== modified file 'tests/unit-tests/client/android/test_client_android_registrar.cpp'
--- tests/unit-tests/client/android/test_client_android_registrar.cpp	2013-06-21 23:04:37 +0000
+++ tests/unit-tests/client/android/test_client_android_registrar.cpp	2013-07-03 17:18:27 +0000
@@ -19,9 +19,12 @@
 #include "src/client/android/android_client_buffer.h"
 #include "src/client/android/android_registrar_gralloc.h"
 #include "mir_test_doubles/mock_android_alloc_device.h"
+#include "mir_test/fake_shared.h"
 
 #include <system/graphics.h>
 #include <stdexcept>
+#include <unistd.h>
+#include <fcntl.h>
 
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
@@ -82,6 +85,7 @@
         return registrar->unlock_interface(module, handle);
     }
 };
+typedef ::testing::NiceMock<MockRegistrarDevice> StubRegistrarDevice;
 
 class ClientAndroidRegistrarTest : public ::testing::Test
 {
@@ -90,7 +94,7 @@
     {
         using namespace testing;
 
-        mock_module = std::make_shared<MockRegistrarDevice>();
+        mock_module = std::make_shared<NiceMock<MockRegistrarDevice>>();
 
         width = 41;
         height = 43;
@@ -158,6 +162,24 @@
     EXPECT_EQ(handle1, handle2);
 }
 
+TEST_F(ClientAndroidRegistrarTest, registrar_frees_fds)
+{
+    using namespace testing;
+    StubRegistrarDevice stub_module;
+    auto package = std::make_shared<MirBufferPackage>();
+    package->data_items = 0;
+    package->fd_items = 2;
+    pipe(static_cast<int*>(package->fd));
+
+    {
+        mcla::AndroidRegistrarGralloc registrar(mir::test::fake_shared(stub_module));
+        auto handle = registrar.register_buffer(package);
+    }
+
+    EXPECT_EQ(-1, fcntl(package->fd[0], F_GETFD));
+    EXPECT_EQ(-1, fcntl(package->fd[1], F_GETFD));
+}
+
 
 TEST_F(ClientAndroidRegistrarTest, register_failure)
 {