Mir

Merge lp:~raof/mir/release-rendersurface-may-require-RPC into lp:mir

release-rendersurface-may-require-RPC
Merge into development-branch

Proposed by Chris Halse Rogers on 2016-12-08

Status:	Work in progress
Proposed branch:	lp:~raof/mir/release-rendersurface-may-require-RPC
Merge into:	lp:mir
Diff against target:	601 lines (+262/-40) 14 files modified include/test/mir/test/validity_matchers.h (+8/-0) playground/egldiamond_render_surface.c (+1/-1) playground/mir_demo_client_chain_jumping_buffers.c (+2/-1) playground/mir_demo_client_prerendered_frames.c (+1/-1) playground/render_surface.cpp (+1/-1) src/client/mir_connection.cpp (+14/-12) src/client/mir_connection.h (+3/-1) src/client/mir_render_surface_api.cpp (+31/-2) src/client/symbols.map (+1/-0) src/include/client/mir_toolkit/mir_render_surface.h (+13/-1) tests/acceptance-tests/staging/test_render_surface.cpp (+147/-16) tests/mir_test/CMakeLists.txt (+4/-0) tests/mir_test/validity_matchers.cpp (+31/-0) tests/unit-tests/client/test_mir_render_surface.cpp (+5/-4)
To merge this branch:	bzr merge lp:~raof/mir/release-rendersurface-may-require-RPC
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Cemil Azizoglu (community)		2016-12-08	Needs Fixing on 2016-12-09
Mir CI Bot	continuous-integration		Approve on 2016-12-08
Review via email: mp+312761@code.launchpad.net

Commit message

Rework mir_render_surface_release() to explicitly identify the RPC wait involved.

In the case that the MirRenderSurface is backed by a MirBufferStream, releasing the MirRenderSurface
implies releasing the MirBufferStream, and releasing the MirBufferStream requires an RPC wait.

Other, future, MirRenderSurface backing objects might require RPC waits to release, too.

Split into the traditional mir_render_surface_release(rs, callback, ctx)/_sync() pairs.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-12-08:

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

review: Needs Fixing (continuous-integration)

lp:~raof/mir/release-rendersurface-may-require-RPC updated on 2016-12-08

3872. By Chris Halse Rogers on 2016-12-08: Fix MirRenderSurfaceTest that make was mysteriously failing to rebuild

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-12-08:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-12-08:

Bug #1647573, retriggering.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-12-08:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-12-08:

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

review: Approve (continuous-integration)

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-12-09:

123 + StreamRelease stream_release{reinterpret_cast<MirBufferStream*>(render_surface),
124 + nullptr,
125 + reinterpret_cast<mir_buffer_stream_callback>(callback),
126 + ctx,
127 rs->stream_id().as_value(),
128 render_surface};

This is actually executed for PCs as well.

review: Needs Fixing

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-12-09:

I guess I don't understand why releasing of streams requires an RPC wait while that of chains doesn't, when the operation is the same (i.e. server.release_buffer_stream(...)).

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-12-12:

Releasing streams requires an RPC wait for two reasons:
1) It's a necessary client synchronisation point: clients almost always want to have some data associated with a Mir object, and they can't free that associated data until we can guarantee they'll never get a callback on that Mir object again.
2) The code
mir_render_surface_release(rs)
mir_connection_release(connection)
is currently racy, and will frequently lead to client deadlocks.

Like so:
  Both the rs_release and connection_release client requests will get queued up
  to the socket, the server sends the responses and then closes the connection.
  .
  The rs_release response results in erasing the bufferstream from the connection
  map.
  .
  The bufferstream destructor calls release_buffer on all of its allocated buffers.
  (This is why the current acceptance tests don't notice the problem - we don't
   wait for the bufferstream to actually be valid, so its allocation requests
   are unfulfilled at destruction time, so it never tries to release them.)
  .
  The buffer_release call hits the RPC layer, which notices that the socket has
  been closed and fires on_disconnected.
  .
  on_disconnected runs all the pending completions. The first one is...
  release_connection, which immediately tries to grab the connection_map
  lock, which we already hold way up in rs_release, and deadlock.

I guess both of these could be solved instead by
a) Ensuring that mir_render_surface_release does not return until it's guaranteed that no more callbacks will be fired on the underlying bufferstream, and
b) Ensuring that we don't deadlock in this scenario.

I'll see how difficult this is to do.

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-12-12:

Hm. (2) is actually a flaw in my follow-up code, so it's only (1) that we're concerned about...

Unmerged revisions

3872. By Chris Halse Rogers on 2016-12-08

Fix MirRenderSurfaceTest that make was mysteriously failing to rebuild

3871. By Chris Halse Rogers on 2016-12-08

Rework mir_render_surface_release() to explicitly identify the RPC wait involved.

In the case that the MirRenderSurface is backed by a MirBufferStream, releasing the MirRenderSurface
implies releasing the MirBufferStream, and releasing the MirBufferStream requires an RPC wait.

Other, future, MirRenderSurface backing objects might require RPC waits to release, too.

Split into the traditional mir_render_surface_release(rs, callback, ctx)/_sync() pairs.

3870. By Chris Halse Rogers on 2016-12-08

Add IsValid() implementations for MirBufferStream and MirRenderSurface

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Chris Halse Rogers

Emanuele Antonio Faraone

Gerry Boland

Mir development team

 === modified file 'include/test/mir/test/validity_matchers.h'
 --- include/test/mir/test/validity_matchers.h	2016-01-29 08:18:22 +0000
 +++ include/test/mir/test/validity_matchers.h	2016-12-08 03:24:08 +0000
@@ -23,6 +23,8 @@
  #include "mir_toolkit/mir_client_library.h"
++class MirRenderSurface;
++
  using ::testing::MakePolymorphicMatcher;
  using ::testing::MatchResultListener;
  using ::testing::NotNull;
@@ -54,6 +56,12 @@
  template<>
  bool IsValidMatcher::MatchAndExplain(MirSurface* surface, MatchResultListener* listener) const;
++template<>
++bool IsValidMatcher::MatchAndExplain(MirBufferStream* stream, MatchResultListener* listener) const;
++
++template<>
++bool IsValidMatcher::MatchAndExplain(MirRenderSurface* stream, MatchResultListener* listener) const;
++
  // To construct a polymorphic matcher, pass an instance of the class
  // to MakePolymorphicMatcher().  Note the return type.
  inline PolymorphicMatcher<IsValidMatcher> IsValid()
 === modified file 'playground/egldiamond_render_surface.c'
 --- playground/egldiamond_render_surface.c	2016-11-30 04:19:26 +0000
 +++ playground/egldiamond_render_surface.c	2016-12-08 03:24:08 +0000
@@ -312,7 +312,7 @@
          future_driver_eglTerminate(egldisplay);
      else
          eglTerminate(egldisplay);
--    mir_render_surface_release(render_surface);
++    mir_render_surface_release_sync(render_surface);
      mir_surface_release_sync(surface);
      mir_connection_release(connection);
      return 0;
 === modified file 'playground/mir_demo_client_chain_jumping_buffers.c'
 --- playground/mir_demo_client_chain_jumping_buffers.c	2016-12-01 16:01:55 +0000
 +++ playground/mir_demo_client_chain_jumping_buffers.c	2016-12-08 03:24:08 +0000
@@ -265,7 +265,8 @@
      for (unsigned int i = 0u; i < num_buffers; i++)
          mir_buffer_release(buffer_available[i].buffer);
      for (unsigned int i = 0u; i < num_chains; i++)
--        mir_render_surface_release(render_surface[i]);
++        mir_render_surface_release_sync(render_surface[i]);
++
      mir_surface_release_sync(surface);
      mir_connection_release(connection);
      return 0;
 === modified file 'playground/mir_demo_client_prerendered_frames.c'
 --- playground/mir_demo_client_prerendered_frames.c	2016-12-01 16:01:55 +0000
 +++ playground/mir_demo_client_prerendered_frames.c	2016-12-08 03:24:08 +0000
@@ -232,7 +232,7 @@
      for (i = 0u; i < num_prerendered_frames; i++)
          mir_buffer_release(buffer_available[i].buffer);
--    mir_render_surface_release(render_surface);
++    mir_render_surface_release_sync(render_surface);
      mir_surface_release_sync(surface);
      mir_connection_release(connection);
      return 0;
 === modified file 'playground/render_surface.cpp'
 --- playground/render_surface.cpp	2016-11-14 22:39:13 +0000
 +++ playground/render_surface.cpp	2016-12-08 03:24:08 +0000
@@ -243,7 +243,7 @@
          mir_buffer_stream_swap_buffers_sync(buffer_stream);
+     }
--    mir_render_surface_release(render_surface);
++    mir_render_surface_release_sync(render_surface);
      mir_surface_release_sync(surface);
      close(signal_watch);
 === modified file 'src/client/mir_connection.cpp'
 --- src/client/mir_connection.cpp	2016-12-06 16:56:33 +0000
 +++ src/client/mir_connection.cpp	2016-12-08 03:24:08 +0000
@@ -475,17 +475,17 @@
  void MirConnection::released(StreamRelease data)
+ {
++    {
++        std::unique_lock<decltype(mutex)> lk(mutex);
++        if (data.rs)
++            surface_map->erase(data.rs);
++        surface_map->erase(mf::BufferStreamId(data.rpc_id));
++    }
++
      if (data.callback)
          data.callback(data.stream, data.context);
      if (data.handle)
          data.handle->result_received();
--
--    {
--        std::unique_lock<decltype(mutex)> lk(mutex);
--        if (data.rs)
--            surface_map->erase(data.rs);
--        surface_map->erase(mf::BufferStreamId(data.rpc_id));
--    }
+ }
  void MirConnection::released(SurfaceRelease data)
@@ -1331,14 +1331,16 @@
+ }
  void MirConnection::release_render_surface_with_content(
--    void* render_surface)
++    void* render_surface,
++    mir_render_surface_callback callback,
++    void* ctx)
+ {
      auto rs = surface_map->render_surface(render_surface);
--    StreamRelease stream_release{nullptr,
--                                 nullptr,
--                                 nullptr,
--                                 nullptr,
++    StreamRelease stream_release{reinterpret_cast<MirBufferStream*>(render_surface),
++                                 nullptr,
++                                 reinterpret_cast<mir_buffer_stream_callback>(callback),
++                                 ctx,
                                   rs->stream_id().as_value(),
                                   render_surface};
 === modified file 'src/client/mir_connection.h'
 --- src/client/mir_connection.h	2016-12-06 16:56:33 +0000
 +++ src/client/mir_connection.h	2016-12-08 03:24:08 +0000
@@ -224,7 +224,9 @@
          void* context,
          void** native_window);
      void release_render_surface_with_content(
--        void* render_surface);
++        void* render_surface,
++        mir_render_surface_callback callback,
++        void* ctx);
      void* request_interface(char const* name, int version);
 === modified file 'src/client/mir_render_surface_api.cpp'
 --- src/client/mir_render_surface_api.cpp	2016-12-06 16:56:33 +0000
 +++ src/client/mir_render_surface_api.cpp	2016-12-08 03:24:08 +0000
@@ -158,19 +158,48 @@
+ }
  void mir_render_surface_release(
--    MirRenderSurface* render_surface)
++    MirRenderSurface* render_surface,
++    mir_render_surface_callback callback,
++    void* ctx)
  try
+ {
      mir::require(render_surface);
      auto connection = connection_map.connection(static_cast<void*>(render_surface));
      connection_map.erase(static_cast<void*>(render_surface));
--    connection->release_render_surface_with_content(render_surface);
++    connection->release_render_surface_with_content(render_surface, callback, ctx);
+ }
  catch (std::exception const& ex)
+ {
++    callback(render_surface, ctx);
      MIR_LOG_UNCAUGHT_EXCEPTION(ex);
+ }
++void mir_render_surface_release_sync(MirRenderSurface* render_surface)
++{
++    struct Waiter
++    {
++        std::condition_variable notifier;
++        std::mutex lock;
++        bool done{false};
++    } waiter;
++
++    mir_render_surface_release(
++        render_surface,
++        [](MirRenderSurface*, void* ctx)
++        {
++            auto waiter = reinterpret_cast<Waiter*>(ctx);
++            {
++                std::lock_guard<std::mutex> lock{waiter->lock};
++                waiter->done = true;
++            }
++            waiter->notifier.notify_all();
++        },
++        &waiter);
++
++    std::unique_lock<std::mutex> lock(waiter.lock);
++    waiter.notifier.wait(lock, [&waiter]() { return waiter.done; });
++}
++
  MirBufferStream* mir_render_surface_get_buffer_stream(
      MirRenderSurface* render_surface,
      int width, int height,
 === modified file 'src/client/symbols.map'
 --- src/client/symbols.map	2016-12-06 16:56:33 +0000
 +++ src/client/symbols.map	2016-12-08 03:24:08 +0000
@@ -396,6 +396,7 @@
      mir_render_surface_get_size;
      mir_render_surface_set_size;
      mir_render_surface_release;
++    mir_render_surface_release_sync;
      mir_surface_spec_add_render_surface;
      #private functions needed temporarily by nested passthrough
 === modified file 'src/include/client/mir_toolkit/mir_render_surface.h'
 --- src/include/client/mir_toolkit/mir_render_surface.h	2016-12-06 16:56:33 +0000
 +++ src/include/client/mir_toolkit/mir_render_surface.h	2016-12-08 03:24:08 +0000
@@ -110,9 +110,21 @@
  /**
   * Release the specified render surface
+  *
-- * \param [in] render_surface                   The render surface to be released
++ * \param [in] render_surface   The render surface to be released
++ * \param [in] callback         Callback to be run when the operation completes
++ * \param [in,out] ctx          Context passed to callback
   */
  void mir_render_surface_release(
++    MirRenderSurface* render_surface,
++    mir_render_surface_callback callback,
++    void* ctx);
++
++/**
++ * Release the specified render surface and wait for the operation to complete
++ *
++ * \param [in] render_surface   The render surface to be released
++ */
++void mir_render_surface_release_sync(
      MirRenderSurface* render_surface);
  /**
 === modified file 'tests/acceptance-tests/staging/test_render_surface.cpp'
 --- tests/acceptance-tests/staging/test_render_surface.cpp	2016-12-06 16:56:33 +0000
 +++ tests/acceptance-tests/staging/test_render_surface.cpp	2016-12-08 03:24:08 +0000
@@ -24,11 +24,13 @@
  #include "mir_test_framework/headless_in_process_server.h"
  #include "mir_test_framework/using_stub_client_platform.h"
  #include "mir/test/validity_matchers.h"
++#include "mir/test/signal.h"
  #include <gtest/gtest.h>
  #include <gmock/gmock.h>
  namespace mtf = mir_test_framework;
++namespace mt = mir::test;
  namespace geom = mir::geometry;
  namespace
@@ -54,7 +56,7 @@
      EXPECT_TRUE(mir_render_surface_is_valid(rs));
      EXPECT_THAT(mir_render_surface_get_error_message(rs), StrEq(""));
--    mir_render_surface_release(rs);
++    mir_render_surface_release_sync(rs);
      mir_connection_release(connection);
+ }
@@ -83,7 +85,7 @@
      EXPECT_TRUE(mir_buffer_stream_is_valid(bs));
      EXPECT_THAT(mir_buffer_stream_get_error_message(bs), StrEq(""));
--    mir_render_surface_release(rs);
++    mir_render_surface_release_sync(rs);
      mir_connection_release(connection);
+ }
@@ -113,7 +115,7 @@
      EXPECT_THAT(bs2, Eq(nullptr));
--    mir_render_surface_release(rs);
++    mir_render_surface_release_sync(rs);
      mir_connection_release(connection);
+ }
@@ -134,7 +136,7 @@
      EXPECT_TRUE(mir_buffer_stream_is_valid(bs));
      EXPECT_THAT(mir_buffer_stream_get_error_message(bs), StrEq(""));
--    mir_render_surface_release(rs);
++    mir_render_surface_release_sync(rs);
      mir_connection_release(connection);
+ }
@@ -157,7 +159,7 @@
      EXPECT_THAT(surface, IsValid());
      EXPECT_THAT(mir_surface_get_buffer_stream(surface), Eq(nullptr));
--    mir_render_surface_release(rs);
++    mir_render_surface_release_sync(rs);
      mir_surface_release_sync(surface);
      mir_connection_release(connection);
+ }
@@ -184,10 +186,11 @@
                                                     usage);
      EXPECT_THAT(surface, IsValid());
++
      EXPECT_THAT(mir_surface_get_buffer_stream(surface), Eq(nullptr));
--    EXPECT_TRUE(mir_buffer_stream_is_valid(bs));
++    EXPECT_THAT(bs, IsValid());
--    mir_render_surface_release(rs);
++    mir_render_surface_release_sync(rs);
      mir_surface_release_sync(surface);
      mir_connection_release(connection);
+ }
@@ -204,7 +207,7 @@
      ASSERT_THAT(pc, NotNull());
      EXPECT_TRUE(mir_presentation_chain_is_valid(pc));
--    mir_render_surface_release(rs);
++    mir_render_surface_release_sync(rs);
      mir_connection_release(connection);
+ }
@@ -224,7 +227,7 @@
      EXPECT_THAT(pc2, Eq(nullptr));
--    mir_render_surface_release(rs);
++    mir_render_surface_release_sync(rs);
      mir_connection_release(connection);
+ }
@@ -249,7 +252,7 @@
      EXPECT_THAT(mir_surface_get_buffer_stream(surface), Eq(nullptr));
      EXPECT_TRUE(mir_presentation_chain_is_valid(pc));
--    mir_render_surface_release(rs);
++    mir_render_surface_release_sync(rs);
      mir_surface_release_sync(surface);
      mir_connection_release(connection);
+ }
@@ -265,7 +268,7 @@
      ASSERT_THAT(pc, NotNull());
      EXPECT_TRUE(mir_presentation_chain_is_valid(pc));
--    mir_render_surface_release(rs);
++    mir_render_surface_release_sync(rs);
      mir_connection_release(connection);
+ }
@@ -289,7 +292,7 @@
      EXPECT_THAT(bs, Eq(nullptr));
--    mir_render_surface_release(rs);
++    mir_render_surface_release_sync(rs);
      mir_connection_release(connection);
+ }
@@ -306,13 +309,141 @@
          mir_pixel_format_abgr_8888,
          mir_buffer_usage_hardware);
--    ASSERT_THAT(bs, NotNull());
--    EXPECT_TRUE(mir_buffer_stream_is_valid(bs));
++    EXPECT_THAT(bs, IsValid());
      auto pc = mir_render_surface_get_presentation_chain(rs);
      EXPECT_THAT(pc, Eq(nullptr));
--    mir_render_surface_release(rs);
--    mir_connection_release(connection);
++    mir_render_surface_release_sync(rs);
++    mir_connection_release(connection);
++}
++
++TEST_F(RenderSurfaceTest, asynchronous_render_surface_destruction_works_with_realised_buffer_stream)
++{
++    auto physical_size = logical_size;
++    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
++
++    auto rs = mir_connection_create_render_surface_sync(
++        connection, logical_size.width.as_int(), logical_size.height.as_int());
++    mir_render_surface_get_buffer_stream(
++        rs,
++        physical_size.width.as_int(), physical_size.height.as_int(),
++        mir_pixel_format_abgr_8888,
++        mir_buffer_usage_hardware);
++
++    struct Context
++    {
++        MirRenderSurface* rs;
++        mt::Signal done;
++    } context;
++
++    context.rs = rs;
++
++    mir_render_surface_release(
++        rs,
++        [](MirRenderSurface* rs, void* ctx)
++        {
++            auto& context = *reinterpret_cast<Context*>(ctx);
++            EXPECT_THAT(rs, Eq(context.rs));
++            /*
++             * Should really test that the render surface isn't valid here, but that fails.
++             *
++            EXPECT_THAT(rs, Not(IsValid()));
++             */
++            context.done.raise();
++        },
++        &context);
++
++    ASSERT_TRUE(context.done.wait_for(std::chrono::seconds{20}));
++
++    mir_connection_release(connection);
++}
++
++TEST_F(RenderSurfaceTest, asynchronous_render_surface_destruction_works_with_realised_presentation_chain)
++{
++    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
++
++    auto rs = mir_connection_create_render_surface_sync(
++        connection, logical_size.width.as_int(), logical_size.height.as_int());
++    mir_render_surface_get_presentation_chain(rs);
++
++    struct Context
++    {
++        MirRenderSurface* rs;
++        mt::Signal done;
++    } context;
++
++    context.rs = rs;
++
++    mir_render_surface_release(
++        rs,
++        [](MirRenderSurface* rs, void* ctx)
++        {
++            auto& context = *reinterpret_cast<Context*>(ctx);
++            EXPECT_THAT(rs, Eq(context.rs));
++            /*
++             * Should really test that the render surface isn't valid here, but that fails.
++             *
++            EXPECT_THAT(rs, Not(IsValid()));
++             */
++            context.done.raise();
++        },
++        &context);
++
++    ASSERT_TRUE(context.done.wait_for(std::chrono::seconds{20}));
++
++    mir_connection_release(connection);
++}
++
++TEST_F(RenderSurfaceTest, asynchronous_render_surface_destruction_works_with_no_backing_object)
++{
++    auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
++
++    auto rs = mir_connection_create_render_surface_sync(
++        connection, logical_size.width.as_int(), logical_size.height.as_int());
++
++    struct Context
++    {
++        MirRenderSurface* rs;
++        mt::Signal done;
++    } context;
++
++    context.rs = rs;
++
++    mir_render_surface_release(
++        rs,
++        [](MirRenderSurface* rs, void* ctx)
++        {
++            auto& context = *reinterpret_cast<Context*>(ctx);
++            EXPECT_THAT(rs, Eq(context.rs));
++            /*
++             * Should really test that the render surface isn't valid here, but that fails.
++             *
++            EXPECT_THAT(rs, Not(IsValid()));
++             */
++            context.done.raise();
++        },
++        &context);
++
++    ASSERT_TRUE(context.done.wait_for(std::chrono::seconds{20}));
++
++    mir_connection_release(connection);
++}
++
++TEST_F(RenderSurfaceTest, asynchronous_destruction_calls_callback_on_failure)
++{
++    auto not_a_valid_client_side_rs = reinterpret_cast<MirRenderSurface*>(0x0ff01bb1);
++
++    mt::Signal callback_called;
++
++    mir_render_surface_release(
++        not_a_valid_client_side_rs,
++        [](MirRenderSurface*, void* ctx)
++        {
++            reinterpret_cast<mt::Signal*>(ctx)->raise();
++        },
++        &callback_called);
++
++    EXPECT_TRUE(callback_called.wait_for(std::chrono::seconds{20}));
+ }
 === modified file 'tests/mir_test/CMakeLists.txt'
 --- tests/mir_test/CMakeLists.txt	2016-11-09 02:30:14 +0000
 +++ tests/mir_test/CMakeLists.txt	2016-12-08 03:24:08 +0000
@@ -2,6 +2,10 @@
  string (REPLACE " -flto " " " CMAKE_C_FLAGS ${CMAKE_C_FLAGS})
  string (REPLACE " -flto " " " CMAKE_CXX_FLAGS ${CMAKE_CXX_FLAGS})
++include_directories(
++  ${PROJECT_SOURCE_DIR}/src/include/client
++)
++
  add_library(mir-public-test OBJECT
    cross_process_action.cpp
    cross_process_sync.cpp
 === modified file 'tests/mir_test/validity_matchers.cpp'
 --- tests/mir_test/validity_matchers.cpp	2015-06-25 03:00:08 +0000
 +++ tests/mir_test/validity_matchers.cpp	2016-12-08 03:24:08 +0000
@@ -1,5 +1,6 @@
  #include "mir/test/validity_matchers.h"
  #include "mir_toolkit/mir_client_library.h"
++#include "mir_toolkit/mir_render_surface.h"
  template<>
  bool IsValidMatcher::MatchAndExplain(MirConnection* connection, MatchResultListener* listener) const
@@ -30,3 +31,33 @@
+     }
      return true;
+ }
++
++template<>
++bool IsValidMatcher::MatchAndExplain(MirBufferStream* stream, MatchResultListener* listener) const
++{
++    if (stream == nullptr)
++    {
++        return false;
++    }
++    if (!mir_buffer_stream_is_valid(stream))
++    {
++        *listener << "error is: " << mir_buffer_stream_get_error_message(stream);
++        return false;
++    }
++    return true;
++}
++
++template<>
++bool IsValidMatcher::MatchAndExplain(MirRenderSurface* rs, MatchResultListener* listener) const
++{
++    if (rs == nullptr)
++    {
++        return false;
++    }
++    if (!mir_render_surface_is_valid(rs))
++    {
++        *listener << "error is: " << mir_render_surface_get_error_message(rs);
++        return false;
++    }
++    return true;
++}
 === modified file 'tests/unit-tests/client/test_mir_render_surface.cpp'
 --- tests/unit-tests/client/test_mir_render_surface.cpp	2016-12-01 18:10:01 +0000
 +++ tests/unit-tests/client/test_mir_render_surface.cpp	2016-12-08 03:24:08 +0000
@@ -44,10 +44,11 @@
  namespace
+ {
--void assign_result(void* result, void** context)
++template<typename T>
++void assign_result(T* result, void* context)
+ {
      if (context)
--        *context = result;
++        *reinterpret_cast<T**>(context) = result;
+ }
  struct RenderSurfaceCallback
@@ -208,14 +209,14 @@
      MirRenderSurface* render_surface = nullptr;
      connection->create_render_surface_with_content(
          {10, 10},
--        reinterpret_cast<mir_render_surface_callback>(assign_result),
++        &assign_result,
          &render_surface,
          &nw);
      EXPECT_THAT(render_surface, NotNull());
      EXPECT_THAT(nw, NotNull());
      EXPECT_THAT(render_surface, Eq(nw));
--    EXPECT_NO_THROW(connection->release_render_surface_with_content(nw));
++    EXPECT_NO_THROW(connection->release_render_surface_with_content(nw, &assign_result, nullptr));
      connection->disconnect();
+ }

Mir

Merge lp:~raof/mir/release-rendersurface-may-require-RPC into lp:mir

Commit message

Description of the change

Unmerged revisions

Preview Diff

Subscribers