Merge lp:~kdub/mir/fix-1664562 into lp:mir

[Kevin DuBois]

nested: make sure not to close the FD given via the fenced_buffers extension. This was causing intermittent problems with mirscreencast on fenced platforms (android)

fixes: LP: #1664562

nested: make sure not to close the FD given via the fenced_buffers extension. This was causing intermittent problems with mirscreencast on fenced platforms (android)

fixes: LP: #1664562

one line of change... line 448 changed to line 194. The rest was extruding classes to their own files to come up with the test.

[Mir CI Bot (mir-ci-bot)]

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

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

review: Approve (continuous-integration)

[Chris Halse Rogers (raof)]

I'd kinda prefer this to be just the one-line change, rather than all the changes to make a test :).

That said, looks fine to me.

review: Approve

[Daniel van Vugt (vanvugt)]

I agree with Chris, although it's also fine putting a larger cleanup on trunk. Only if you were backporting the fix to a stable branch like lp:mir/0.26 then you should choose the one-liner solution over this.

review: Abstain

=== modified file 'src/server/graphics/nested/CMakeLists.txt'
--- src/server/graphics/nested/CMakeLists.txt	2017-01-18 02:29:37 +0000
+++ src/server/graphics/nested/CMakeLists.txt	2017-02-14 17:05:00 +0000
@@ -15,4 +15,5 @@
   platform.cpp
   buffer.cpp
   ipc_operations.cpp
+  host_buffer.cpp
 )

=== added file 'src/server/graphics/nested/host_buffer.cpp'
--- src/server/graphics/nested/host_buffer.cpp	1970-01-01 00:00:00 +0000
+++ src/server/graphics/nested/host_buffer.cpp	2017-02-14 17:05:00 +0000
@@ -0,0 +1,183 @@
+/*
+ * Copyright © 2017 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Kevin DuBois <[e-mail]>
+ */
+
+#include "host_buffer.h"
+#include "mir_toolkit/mir_buffer_private.h"
+#include <boost/throw_exception.hpp>
+
+namespace mg = mir::graphics;
+namespace mgn = mir::graphics::nested;
+namespace geom = mir::geometry;
+
+mgn::HostBuffer::HostBuffer(MirConnection* mir_connection, mg::BufferProperties const& properties) :
+    fence_extensions(mir_extension_fenced_buffers_v1(mir_connection))
+{
+    mir_connection_allocate_buffer(
+        mir_connection,
+        properties.size.width.as_int(),
+        properties.size.height.as_int(),
+        properties.format,
+        buffer_available, this);
+    std::unique_lock<std::mutex> lk(mut);
+    cv.wait(lk, [&]{ return handle; });
+    if (!mir_buffer_is_valid(handle))
+    {
+        mir_buffer_release(handle);
+        BOOST_THROW_EXCEPTION(std::runtime_error("could not allocate MirBuffer"));
+    }
+}
+
+mgn::HostBuffer::HostBuffer(MirConnection* mir_connection, geom::Size size, MirPixelFormat format) :
+    fence_extensions(mir_extension_fenced_buffers_v1(mir_connection))
+{
+    mir_connection_allocate_buffer(
+        mir_connection,
+        size.width.as_int(),
+        size.height.as_int(),
+        format,
+        buffer_available, this);
+    std::unique_lock<std::mutex> lk(mut);
+    cv.wait(lk, [&]{ return handle; });
+    if (!mir_buffer_is_valid(handle))
+    {
+        mir_buffer_release(handle);
+        BOOST_THROW_EXCEPTION(std::runtime_error("could not allocate MirBuffer"));
+    }
+}
+
+mgn::HostBuffer::HostBuffer(MirConnection* mir_connection,
+    MirExtensionGbmBufferV1 const* ext,
+    geom::Size size, unsigned int native_pf, unsigned int native_flags) :
+    fence_extensions(mir_extension_fenced_buffers_v1(mir_connection))
+{
+    ext->allocate_buffer_gbm(
+        mir_connection, size.width.as_int(), size.height.as_int(), native_pf, native_flags,
+        buffer_available, this);
+    std::unique_lock<std::mutex> lk(mut);
+    cv.wait(lk, [&]{ return handle; });
+    if (!mir_buffer_is_valid(handle))
+    {
+        mir_buffer_release(handle);
+        BOOST_THROW_EXCEPTION(std::runtime_error("could not allocate MirBuffer"));
+    }
+}
+
+mgn::HostBuffer::HostBuffer(MirConnection* mir_connection,
+    MirExtensionAndroidBufferV1 const* ext,
+    geom::Size size, unsigned int native_pf, unsigned int native_flags) :
+    fence_extensions(mir_extension_fenced_buffers_v1(mir_connection))
+{
+    ext->allocate_buffer_android(
+        mir_connection, size.width.as_int(), size.height.as_int(), native_pf, native_flags,
+        buffer_available, this);
+    std::unique_lock<std::mutex> lk(mut);
+    cv.wait(lk, [&]{ return handle; });
+    if (!mir_buffer_is_valid(handle))
+    {
+        mir_buffer_release(handle);
+        BOOST_THROW_EXCEPTION(std::runtime_error("could not allocate MirBuffer"));
+    }
+}
+
+mgn::HostBuffer::~HostBuffer()
+{
+    mir_buffer_release(handle);
+}
+
+void mgn::HostBuffer::sync(MirBufferAccess access, std::chrono::nanoseconds ns)
+{
+    if (fence_extensions && fence_extensions->wait_for_access)
+        fence_extensions->wait_for_access(handle, access, ns.count());
+}
+
+MirBuffer* mgn::HostBuffer::client_handle() const
+{
+    return handle;
+}
+
+std::unique_ptr<mgn::GraphicsRegion> mgn::HostBuffer::get_graphics_region()
+{
+    return std::make_unique<mgn::GraphicsRegion>(handle);
+}
+
+geom::Size mgn::HostBuffer::size() const
+{
+    return { mir_buffer_get_width(handle), mir_buffer_get_height(handle) };
+}
+
+MirPixelFormat mgn::HostBuffer::format() const
+{
+    return mir_buffer_get_pixel_format(handle);
+}
+
+std::tuple<EGLenum, EGLClientBuffer, EGLint*> mgn::HostBuffer::egl_image_creation_hints() const
+{
+    EGLenum type;
+    EGLClientBuffer client_buffer = nullptr;;
+    EGLint* attrs = nullptr;
+    // TODO: check return value
+    mir_buffer_get_egl_image_parameters(handle, &type, &client_buffer, &attrs);
+    
+    return std::tuple<EGLenum, EGLClientBuffer, EGLint*>{type, client_buffer, attrs};
+}
+
+void mgn::HostBuffer::buffer_available(MirBuffer* buffer, void* context)
+{
+    auto host_buffer = static_cast<HostBuffer*>(context);
+    host_buffer->available(buffer);
+}
+
+void mgn::HostBuffer::available(MirBuffer* buffer)
+{
+    std::unique_lock<std::mutex> lk(mut);
+    if (!handle)
+    {
+        handle = buffer;
+        cv.notify_all();
+    }
+
+    auto g = f;
+    lk.unlock();
+    if (g)
+        g();
+}
+
+void mgn::HostBuffer::on_ownership_notification(std::function<void()> const& fn)
+{
+    std::unique_lock<std::mutex> lk(mut);
+    f = fn;
+}
+
+MirBufferPackage* mgn::HostBuffer::package() const
+{
+   return mir_buffer_get_buffer_package(handle);
+}
+
+void mgn::HostBuffer::set_fence(mir::Fd fd)
+{
+    if (fence_extensions && fence_extensions->associate_fence)
+        fence_extensions->associate_fence(handle, fd, mir_read_write);
+}
+
+mir::Fd mgn::HostBuffer::fence() const
+{
+    if (fence_extensions && fence_extensions->get_fence)
+        return mir::Fd{mir::IntOwnedFd{fence_extensions->get_fence(handle)}};
+    else
+        return mir::Fd{mir::Fd::invalid};
+}

=== added file 'src/server/graphics/nested/host_buffer.h'
--- src/server/graphics/nested/host_buffer.h	1970-01-01 00:00:00 +0000
+++ src/server/graphics/nested/host_buffer.h	2017-02-14 17:05:00 +0000
@@ -0,0 +1,73 @@
+/*
+ * Copyright © 2017 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Kevin DuBois <[e-mail]>
+ */
+
+#ifndef MIR_GRAPHICS_NESTED_HOST_BUFFER_H_
+#define MIR_GRAPHICS_NESTED_HOST_BUFFER_H_
+
+#include "mir/graphics/buffer_properties.h"
+#include "mir_toolkit/mir_buffer.h"
+#include "mir_toolkit/extensions/gbm_buffer.h"
+#include "mir_toolkit/extensions/android_buffer.h"
+#include "native_buffer.h"
+#include <functional>
+#include <mutex>
+#include <condition_variable>
+
+namespace mir
+{
+namespace graphics
+{
+namespace nested
+{
+class HostBuffer : public NativeBuffer
+{
+public:
+    HostBuffer(MirConnection* mir_connection, BufferProperties const& properties);
+    HostBuffer(MirConnection* mir_connection, geometry::Size size, MirPixelFormat format);
+    HostBuffer(MirConnection* mir_connection,
+        MirExtensionGbmBufferV1 const* ext,
+        geometry::Size size, unsigned int native_pf, unsigned int native_flags);
+    HostBuffer(MirConnection* mir_connection,
+        MirExtensionAndroidBufferV1 const* ext,
+        geometry::Size size, unsigned int native_pf, unsigned int native_flags);
+    ~HostBuffer();
+
+    void sync(MirBufferAccess access, std::chrono::nanoseconds ns) override;
+    MirBuffer* client_handle() const override;
+    std::unique_ptr<GraphicsRegion> get_graphics_region() override;
+    geometry::Size size() const override;
+    MirPixelFormat format() const override;
+    std::tuple<EGLenum, EGLClientBuffer, EGLint*> egl_image_creation_hints() const override;
+    static void buffer_available(MirBuffer* buffer, void* context);
+    void available(MirBuffer* buffer) override;
+    void on_ownership_notification(std::function<void()> const& fn) override;
+    MirBufferPackage* package() const override;
+    void set_fence(mir::Fd fd) override;
+    mir::Fd fence() const override;
+
+private:
+    MirExtensionFencedBuffersV1 const* fence_extensions = nullptr;
+    std::function<void()> f;
+    MirBuffer* handle = nullptr;
+    std::mutex mut;
+    std::condition_variable cv;
+};
+}
+}
+}
+#endif /* MIR_GRAPHICS_NESTED_HOST_BUFFER_H_ */

=== modified file 'src/server/graphics/nested/mir_client_host_connection.cpp'
--- src/server/graphics/nested/mir_client_host_connection.cpp	2017-02-14 07:05:13 +0000
+++ src/server/graphics/nested/mir_client_host_connection.cpp	2017-02-14 17:05:00 +0000
@@ -21,6 +21,7 @@
 #include "host_stream.h"
 #include "host_chain.h"
 #include "host_surface_spec.h"
+#include "host_buffer.h"
 #include "native_buffer.h"
 #include "mir_toolkit/mir_client_library.h"
 #include "mir_toolkit/mir_extension_core.h"
@@ -610,176 +611,6 @@
 
 namespace
 {
-class HostBuffer : public mgn::NativeBuffer
-{
-public:
-    HostBuffer(MirConnection* mir_connection, mg::BufferProperties const& properties) :
-        fence_extensions(mir_extension_fenced_buffers_v1(mir_connection))
-    {
-        mir_connection_allocate_buffer(
-            mir_connection,
-            properties.size.width.as_int(),
-            properties.size.height.as_int(),
-            properties.format,
-            buffer_available, this);
-        std::unique_lock<std::mutex> lk(mut);
-        cv.wait(lk, [&]{ return handle; });
-        if (!mir_buffer_is_valid(handle))
-        {
-            mir_buffer_release(handle);
-            BOOST_THROW_EXCEPTION(std::runtime_error("could not allocate MirBuffer"));
-        }
-    }
-
-    HostBuffer(MirConnection* mir_connection, geom::Size size, MirPixelFormat format) :
-        fence_extensions(mir_extension_fenced_buffers_v1(mir_connection))
-    {
-        mir_connection_allocate_buffer(
-            mir_connection,
-            size.width.as_int(),
-            size.height.as_int(),
-            format,
-            buffer_available, this);
-        std::unique_lock<std::mutex> lk(mut);
-        cv.wait(lk, [&]{ return handle; });
-        if (!mir_buffer_is_valid(handle))
-        {
-            mir_buffer_release(handle);
-            BOOST_THROW_EXCEPTION(std::runtime_error("could not allocate MirBuffer"));
-        }
-    }
-
-    HostBuffer(MirConnection* mir_connection,
-        MirExtensionGbmBufferV1 const* ext,
-        geom::Size size, unsigned int native_pf, unsigned int native_flags) :
-        fence_extensions(mir_extension_fenced_buffers_v1(mir_connection))
-    {
-        ext->allocate_buffer_gbm(
-            mir_connection, size.width.as_int(), size.height.as_int(), native_pf, native_flags,
-            buffer_available, this);
-        std::unique_lock<std::mutex> lk(mut);
-        cv.wait(lk, [&]{ return handle; });
-        if (!mir_buffer_is_valid(handle))
-        {
-            mir_buffer_release(handle);
-            BOOST_THROW_EXCEPTION(std::runtime_error("could not allocate MirBuffer"));
-        }
-    }
-
-    HostBuffer(MirConnection* mir_connection,
-        MirExtensionAndroidBufferV1 const* ext,
-        geom::Size size, unsigned int native_pf, unsigned int native_flags) :
-        fence_extensions(mir_extension_fenced_buffers_v1(mir_connection))
-    {
-        ext->allocate_buffer_android(
-            mir_connection, size.width.as_int(), size.height.as_int(), native_pf, native_flags,
-            buffer_available, this);
-        std::unique_lock<std::mutex> lk(mut);
-        cv.wait(lk, [&]{ return handle; });
-        if (!mir_buffer_is_valid(handle))
-        {
-            mir_buffer_release(handle);
-            BOOST_THROW_EXCEPTION(std::runtime_error("could not allocate MirBuffer"));
-        }
-    }
-
-    ~HostBuffer()
-    {
-        mir_buffer_release(handle);
-    }
-
-    void sync(MirBufferAccess access, std::chrono::nanoseconds ns) override
-    {
-        if (fence_extensions && fence_extensions->wait_for_access)
-            fence_extensions->wait_for_access(handle, access, ns.count());
-    }
-
-    MirBuffer* client_handle() const override
-    {
-        return handle;
-    }
-
-    std::unique_ptr<mgn::GraphicsRegion> get_graphics_region() override
-    {
-        return std::make_unique<mgn::GraphicsRegion>(handle);
-    }
-
-    geom::Size size() const override
-    {
-        return { mir_buffer_get_width(handle), mir_buffer_get_height(handle) };
-    }
-
-    MirPixelFormat format() const override
-    {
-        return mir_buffer_get_pixel_format(handle);
-    }
-
-    std::tuple<EGLenum, EGLClientBuffer, EGLint*> egl_image_creation_hints() const override
-    {
-        EGLenum type;
-        EGLClientBuffer client_buffer = nullptr;;
-        EGLint* attrs = nullptr;
-        // TODO: check return value
-        mir_buffer_get_egl_image_parameters(handle, &type, &client_buffer, &attrs);
-        
-        return std::tuple<EGLenum, EGLClientBuffer, EGLint*>{type, client_buffer, attrs};
-    }
-
-    static void buffer_available(MirBuffer* buffer, void* context)
-    {
-        auto host_buffer = static_cast<HostBuffer*>(context);
-        host_buffer->available(buffer);
-    }
-
-    void available(MirBuffer* buffer) override
-    {
-        std::unique_lock<std::mutex> lk(mut);
-        if (!handle)
-        {
-            handle = buffer;
-            cv.notify_all();
-        }
-
-        auto g = f;
-        lk.unlock();
-        if (g)
-            g();
-    }
-
-    void on_ownership_notification(std::function<void()> const& fn) override
-    {
-        std::unique_lock<std::mutex> lk(mut);
-        f = fn;
-    }
-
-    MirBufferPackage* package() const override
-    {
-       return mir_buffer_get_buffer_package(handle);
-    }
-
-    void set_fence(mir::Fd fd) override
-    {
-        if (fence_extensions && fence_extensions->associate_fence)
-            fence_extensions->associate_fence(handle, fd, mir_read_write);
-    }
-
-    mir::Fd fence() const override
-    {
-        if (fence_extensions && fence_extensions->get_fence)
-            return mir::Fd{fence_extensions->get_fence(handle)};
-        else
-            return mir::Fd{mir::Fd::invalid};
-    }
-
-private:
-    MirExtensionFencedBuffersV1 const* fence_extensions = nullptr;
-
-    std::function<void()> f;
-    MirBuffer* handle = nullptr;
-    std::mutex mut;
-    std::condition_variable cv;
-};
-
 class SurfaceSpec : public mgn::HostSurfaceSpec
 {
 public:
@@ -824,13 +655,13 @@
 std::shared_ptr<mgn::NativeBuffer> mgn::MirClientHostConnection::create_buffer(
     mg::BufferProperties const& properties)
 {
-    return std::make_shared<HostBuffer>(mir_connection, properties);
+    return std::make_shared<mgn::HostBuffer>(mir_connection, properties);
 }
 
 std::shared_ptr<mgn::NativeBuffer> mgn::MirClientHostConnection::create_buffer(
     geom::Size size, MirPixelFormat format)
 {
-    return std::make_shared<HostBuffer>(mir_connection, size, format);
+    return std::make_shared<mgn::HostBuffer>(mir_connection, size, format);
 }
 
 std::shared_ptr<mgn::NativeBuffer> mgn::MirClientHostConnection::create_buffer(
@@ -838,12 +669,12 @@
 {
     if (auto ext = mir_extension_gbm_buffer_v1(mir_connection))
     {
-        return std::make_shared<HostBuffer>(mir_connection, ext, size, native_format, native_flags);
+        return std::make_shared<mgn::HostBuffer>(mir_connection, ext, size, native_format, native_flags);
     }
 
     if (auto ext = mir_extension_android_buffer_v1(mir_connection))
     {
-        return std::make_shared<HostBuffer>(mir_connection, ext, size, native_format, native_flags);
+        return std::make_shared<mgn::HostBuffer>(mir_connection, ext, size, native_format, native_flags);
     }
 
     BOOST_THROW_EXCEPTION(std::runtime_error("could not create hardware buffer"));

=== added file 'tests/include/mir/test/doubles/mock_client_platform.h'
--- tests/include/mir/test/doubles/mock_client_platform.h	1970-01-01 00:00:00 +0000
+++ tests/include/mir/test/doubles/mock_client_platform.h	2017-02-14 17:05:00 +0000
@@ -0,0 +1,76 @@
+/*
+ * Copyright © 2017 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Kevin DuBois <[e-mail]>
+ */
+
+#ifndef MIR_TEST_DOUBLES_MOCK_CLIENT_PLATFOM_H_
+#define MIR_TEST_DOUBLES_MOCK_CLIENT_PLATFOM_H_
+
+#include "stub_client_buffer_factory.h"
+#include "mir/client_platform.h"
+#include "mir/client_platform_factory.h"
+#include <gmock/gmock.h>
+
+namespace mir
+{
+namespace test
+{
+namespace doubles
+{
+
+struct MockClientPlatform : public client::ClientPlatform
+{
+    MockClientPlatform()
+    {
+        auto native_window = std::make_shared<EGLNativeWindowType>();
+        *native_window = reinterpret_cast<EGLNativeWindowType>(this);
+
+        ON_CALL(*this, create_buffer_factory())
+            .WillByDefault(testing::Return(std::make_shared<StubClientBufferFactory>()));
+        ON_CALL(*this, create_egl_native_window(testing::_))
+            .WillByDefault(testing::Return(native_window));
+    }
+
+    void set_client_context(client::ClientContext* ctx)
+    {
+        client_context = ctx;
+    }
+
+    void populate(MirPlatformPackage& pkg) const override
+    {
+        client_context->populate_server_package(pkg);
+    }
+
+    MOCK_CONST_METHOD1(convert_native_buffer, MirNativeBuffer*(mir::graphics::NativeBuffer*));
+    MOCK_CONST_METHOD0(platform_type, MirPlatformType());
+    MOCK_METHOD1(platform_operation, MirPlatformMessage*(MirPlatformMessage const*));
+    MOCK_METHOD0(create_buffer_factory, std::shared_ptr<client::ClientBufferFactory>());
+    MOCK_METHOD2(use_egl_native_window, void(std::shared_ptr<void>, client::EGLNativeSurface*));
+    MOCK_METHOD1(create_egl_native_window, std::shared_ptr<void>(client::EGLNativeSurface*));
+    MOCK_METHOD0(create_egl_native_display, std::shared_ptr<EGLNativeDisplayType>());
+    MOCK_CONST_METHOD2(get_egl_pixel_format, MirPixelFormat(EGLDisplay, EGLConfig));
+    MOCK_METHOD2(request_interface, void*(char const*, int));
+    MOCK_CONST_METHOD1(native_format_for, uint32_t(MirPixelFormat));
+    MOCK_CONST_METHOD2(native_flags_for, uint32_t(MirBufferUsage, mir::geometry::Size));
+
+    client::ClientContext* client_context = nullptr;
+};
+
+}
+}
+}
+
+#endif /* MIR_TEST_DOUBLES_MOCK_CLIENT_PLATFOM_H_ */

=== added file 'tests/include/mir/test/doubles/stub_client_platform_factory.h'
--- tests/include/mir/test/doubles/stub_client_platform_factory.h	1970-01-01 00:00:00 +0000
+++ tests/include/mir/test/doubles/stub_client_platform_factory.h	2017-02-14 17:05:00 +0000
@@ -0,0 +1,50 @@
+/*
+ * Copyright © 2017 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Kevin DuBois <[e-mail]>
+ */
+
+#ifndef MIR_TEST_DOUBLES_STUB_CLIENT_PLATFORM_FACTORY_H_
+#define MIR_TEST_DOUBLES_STUB_CLIENT_PLATFORM_FACTORY_H_
+
+#include "mir/client_platform_factory.h"
+
+namespace mir
+{
+namespace test
+{
+namespace doubles
+{
+
+struct StubClientPlatformFactory : public client::ClientPlatformFactory
+{
+    StubClientPlatformFactory(std::shared_ptr<client::ClientPlatform> const& platform)
+        : platform{platform}
+    {
+    }
+
+    std::shared_ptr<client::ClientPlatform> create_client_platform(client::ClientContext*)
+    {
+        return platform;
+    }
+
+    std::shared_ptr<client::ClientPlatform> platform;
+};
+
+}
+}
+}
+
+#endif /* MIR_TEST_DOUBLES_STUB_CLIENT_PLATFORM_FACTORY_H_ */

=== modified file 'tests/include/mir/test/stub_server_tool.h'
--- tests/include/mir/test/stub_server_tool.h	2017-02-14 07:05:13 +0000
+++ tests/include/mir/test/stub_server_tool.h	2017-02-14 17:05:00 +0000
@@ -123,6 +123,20 @@
     {
         done->Run();
     }
+    virtual void allocate_buffers(
+        mir::protobuf::BufferAllocation const* /*request*/,
+        mir::protobuf::Void* /*response*/,
+        google::protobuf::Closure* done)
+    {
+        done->Run();
+    }
+    void release_buffers(
+        mir::protobuf::BufferRelease const* /*request*/,
+        mir::protobuf::Void* /*response*/,
+        google::protobuf::Closure* done)
+    {
+        done->Run();
+    }
 
     std::string application_name() const
     {

=== modified file 'tests/unit-tests/client/test_mir_render_surface.cpp'
--- tests/unit-tests/client/test_mir_render_surface.cpp	2017-02-07 07:13:47 +0000
+++ tests/unit-tests/client/test_mir_render_surface.cpp	2017-02-14 17:05:00 +0000
@@ -28,6 +28,8 @@
 
 #include "mir/test/fake_shared.h"
 #include "mir/test/doubles/stub_client_buffer_factory.h"
+#include "mir/test/doubles/stub_client_platform_factory.h"
+#include "mir/test/doubles/mock_client_platform.h"
 #include "mir_protobuf.pb.h"
 
 #include <sys/eventfd.h>
@@ -102,59 +104,6 @@
     mir::Fd pollable_fd;
 };
 
-struct MockClientPlatform : public mcl::ClientPlatform
-{
-    MockClientPlatform()
-    {
-        auto native_window = std::make_shared<EGLNativeWindowType>();
-        *native_window = reinterpret_cast<EGLNativeWindowType>(this);
-
-        ON_CALL(*this, create_buffer_factory())
-            .WillByDefault(Return(std::make_shared<mtd::StubClientBufferFactory>()));
-        ON_CALL(*this, create_egl_native_window(_))
-            .WillByDefault(Return(native_window));
-    }
-
-    void set_client_context(mcl::ClientContext* ctx)
-    {
-        client_context = ctx;
-    }
-
-    void populate(MirPlatformPackage& pkg) const override
-    {
-        client_context->populate_server_package(pkg);
-    }
-
-    MOCK_CONST_METHOD1(convert_native_buffer, MirNativeBuffer*(mir::graphics::NativeBuffer*));
-    MOCK_CONST_METHOD0(platform_type, MirPlatformType());
-    MOCK_METHOD1(platform_operation, MirPlatformMessage*(MirPlatformMessage const*));
-    MOCK_METHOD0(create_buffer_factory, std::shared_ptr<mcl::ClientBufferFactory>());
-    MOCK_METHOD2(use_egl_native_window, void(std::shared_ptr<void>, mcl::EGLNativeSurface*));
-    MOCK_METHOD1(create_egl_native_window, std::shared_ptr<void>(mcl::EGLNativeSurface*));
-    MOCK_METHOD0(create_egl_native_display, std::shared_ptr<EGLNativeDisplayType>());
-    MOCK_CONST_METHOD2(get_egl_pixel_format, MirPixelFormat(EGLDisplay, EGLConfig));
-    MOCK_METHOD2(request_interface, void*(char const*, int));
-    MOCK_CONST_METHOD1(native_format_for, uint32_t(MirPixelFormat));
-    MOCK_CONST_METHOD2(native_flags_for, uint32_t(MirBufferUsage, mir::geometry::Size));
-
-    mcl::ClientContext* client_context = nullptr;
-};
-
-struct StubClientPlatformFactory : public mcl::ClientPlatformFactory
-{
-    StubClientPlatformFactory(std::shared_ptr<mcl::ClientPlatform> const& platform)
-        : platform{platform}
-    {
-    }
-
-    std::shared_ptr<mcl::ClientPlatform> create_client_platform(mcl::ClientContext*)
-    {
-        return platform;
-    }
-
-    std::shared_ptr<mcl::ClientPlatform> platform;
-};
-
 void connected_callback(MirConnection* /*connection*/, void* /*client_context*/)
 {
 }
@@ -178,7 +127,7 @@
 
     std::shared_ptr<mcl::ClientPlatformFactory> the_client_platform_factory() override
     {
-        return std::make_shared<StubClientPlatformFactory>(platform);
+        return std::make_shared<mtd::StubClientPlatformFactory>(platform);
     }
 
 private:
@@ -190,7 +139,7 @@
 struct MirRenderSurfaceTest : public testing::Test
 {
     MirRenderSurfaceTest()
-        : mock_platform{std::make_shared<testing::NiceMock<MockClientPlatform>>()},
+        : mock_platform{std::make_shared<testing::NiceMock<mtd::MockClientPlatform>>()},
           mock_channel{std::make_shared<testing::NiceMock<MockRpcChannel>>()},
           conf{mock_platform, mock_channel},
           connection{std::make_shared<MirConnection>(conf)}
@@ -198,7 +147,7 @@
         mock_platform->set_client_context(connection.get());
     }
 
-    std::shared_ptr<testing::NiceMock<MockClientPlatform>> const mock_platform;
+    std::shared_ptr<testing::NiceMock<mtd::MockClientPlatform>> const mock_platform;
     std::shared_ptr<testing::NiceMock<MockRpcChannel>> const mock_channel;
     TestConnectionConfiguration conf;
     std::shared_ptr<MirConnection> const connection;

=== modified file 'tests/unit-tests/platforms/nested/CMakeLists.txt'
--- tests/unit-tests/platforms/nested/CMakeLists.txt	2017-01-30 08:13:20 +0000
+++ tests/unit-tests/platforms/nested/CMakeLists.txt	2017-02-14 17:05:00 +0000
@@ -6,6 +6,7 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/test_buffer.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_nested_display_buffer.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_ipc_operations.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_host_buffer.cpp
   $<TARGET_OBJECTS:mir-test-doubles-udev>
   ${MIR_PLATFORM_OBJECTS}
   ${MIR_SERVER_OBJECTS}

=== added file 'tests/unit-tests/platforms/nested/test_host_buffer.cpp'
--- tests/unit-tests/platforms/nested/test_host_buffer.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/platforms/nested/test_host_buffer.cpp	2017-02-14 17:05:00 +0000
@@ -0,0 +1,127 @@
+/*
+ * Copyright © 2017 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Kevin DuBois <[e-mail]>
+ */
+
+#include "src/server/graphics/nested/host_buffer.h"
+#include "src/client/default_connection_configuration.h"
+#include "src/client/mir_connection.h"
+#include "src/client/buffer_factory.h"
+#include "src/client/buffer.h"
+#include "mir/test/stub_server_tool.h"
+#include "mir/test/test_protobuf_server.h"
+#include "mir/test/doubles/stub_client_platform_factory.h"
+#include "mir/test/doubles/mock_client_platform.h"
+#include "mir/frontend/connector.h"
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+#include <fcntl.h>
+
+using namespace testing;
+namespace mgn = mir::graphics::nested;
+namespace mt = mir::test;
+namespace mcl = mir::client;
+namespace mtd = mir::test::doubles;
+
+std::unique_ptr<mir::Fd> fd = nullptr;
+int get_fence(MirBuffer*)
+{
+    if (fd)
+        return *fd;
+    return -1;
+}
+
+struct TestConnectionConfiguration : mir::client::DefaultConnectionConfiguration
+{
+public:
+
+    TestConnectionConfiguration(std::string const& socket)
+        : DefaultConnectionConfiguration(socket)
+    {
+    }
+
+    std::shared_ptr<mcl::ClientPlatformFactory> the_client_platform_factory() override
+    {
+        auto platform = std::make_shared<mtd::MockClientPlatform>();
+        ON_CALL(*platform, request_interface(StrEq("mir_extension_fenced_buffers"), 1))
+            .WillByDefault(Return(&fence_ext));
+        return std::make_shared<mtd::StubClientPlatformFactory>(platform);
+    }
+
+    MirExtensionFencedBuffersV1 fence_ext{get_fence, nullptr, nullptr};
+
+    std::shared_ptr<mir::client::AsyncBufferFactory> the_buffer_factory() override
+    {
+        struct StubBufferFactory : mcl::AsyncBufferFactory
+        {
+            std::shared_ptr<mcl::Buffer> buffer;
+            std::unique_ptr<mcl::MirBuffer> generate_buffer(mir::protobuf::Buffer const&) override
+            {
+                return nullptr;
+            }
+            void expect_buffer(
+                std::shared_ptr<mcl::ClientBufferFactory> const&, 
+                MirConnection* conn, mir::geometry::Size,
+                MirPixelFormat, MirBufferUsage usage, MirBufferCallback cb, void* ctx) override
+            {
+                if (!buffer)
+                    buffer = std::make_shared<mcl::Buffer>(cb, ctx, 1, nullptr, conn, usage);
+                cb(reinterpret_cast<MirBuffer*>(buffer.get()), ctx);
+            }
+            void expect_buffer(
+                std::shared_ptr<mcl::ClientBufferFactory> const&,
+                MirConnection*, mir::geometry::Size, uint32_t, uint32_t,
+                MirBufferCallback, void*) override
+            {
+            }
+            void cancel_requests_with_context(void*) {}
+        };
+        return std::make_shared<StubBufferFactory>(); 
+    }
+};
+
+struct HostBuffer : Test
+{
+    HostBuffer()
+    {
+        //MirConnection is hard to stub
+        std::remove(socket.c_str());
+        test_server = std::make_shared<mt::TestProtobufServer>(socket, server_tool);
+        test_server->comm->start();
+        config = std::make_shared<TestConnectionConfiguration>(socket);
+    }
+
+    std::string const socket { "./test_sock" };
+    std::shared_ptr<mt::StubServerTool> const server_tool = std::make_shared<mt::StubServerTool>();
+    std::shared_ptr<mt::TestProtobufServer> test_server;
+    std::shared_ptr<TestConnectionConfiguration> config;
+};
+
+//LP: #1664562
+TEST_F(HostBuffer, does_not_own_fd_when_accessing_fence)
+{
+    fd = std::make_unique<mir::Fd>( fileno(tmpfile()) );
+
+    MirConnection connection{*config};
+    connection.connect("", [](auto*, auto*){}, nullptr)->wait_for_all();
+    mgn::HostBuffer buffer(&connection, mir::geometry::Size{1,1}, mir_pixel_format_abgr_8888);
+    {
+        auto fence = buffer.fence();
+        EXPECT_THAT(fence, Eq(*fd));
+    }
+    EXPECT_THAT(fcntl(*fd, F_GETFD), Eq(0));
+    fd.reset();
+}