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

[Alberto Aguirre]

Avoid allocating mir::protobuf objects on the stack

When an addition is made to a protobuf message, a stack allocation of such object may differ from what the destructor of that message is expecting as the destructor is defined by libmirprotobuf and the allocation may have been made from an older definition of the message (by an older mirclient library for example) which can lead to stack corruption.

Ideally, the mirprotobuf library would have versioned symbols and have the ability to be loaded in parallel to a previous version but that's not currently possible.

As an alternative, avoid allocating mir defined protobuf objects on the stack and instead use the xxx::default_instance().New() factory methods which are defined in the mirprotobuf library.

Avoid allocating mir::protobuf objects on the stack

When an addition is made to a protobuf message, a stack allocation of such object may differ from what the destructor of that message is expecting as the destructor is defined by libmirprotobuf and the allocation may have been made from an older definition of the message (by an older mirclient library for example) which can lead to stack corruption.

Ideally, the mirprotobuf library would have versioned symbols and have the ability to be loaded in parallel to a previous version but that's not currently possible.

As an alternative, avoid allocating mir defined protobuf objects on the stack and instead use the xxx::default_instance().New() factory methods which are defined in the mirprotobuf library.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:2671
http://jenkins.qa.ubuntu.com/job/mir-ci/4120/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/2880
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/393
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/2828
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/276
        deb: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/276/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2828
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/2828/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5658
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21235

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

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

Slightly scary no one figured this out earlier.

A worse way to think of the bug is: If a new field is added to a message then any existing code will try to construct that message on the stack using its old (smaller) size while Protobuf itself fills out the new fields (which we didn't allow for in the stack allocation). Hence stack corruption.

The solution presented here however fails to work in all cases of ABI breaks such as when a member is removed from the middle of the object. Not likely, but there is a better option...

The issue is really that we're breaking MIRPROTOBUF_ABI but have never thought to bump it. If it was bumped any any change to message sizes/structure then everyone would always be linked to a version that is safe for them. And bumping MIRPROTOBUF_ABI would mean we could keep our nice fast stack variables.

review: Needs Information

[Daniel van Vugt (vanvugt)]

So I think Disapprove/Needs fixing. A simple bump of MIRPROTOBUF_ABI will solve it more reliably and give us more efficient code. Unless anyone can prove there's a protobuf issue preventing us from doing that.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

It's also worth noting you don't need versioned symbols at all if your major ABI number is changing instead.

[Chris Halse Rogers (raof)]

There *is* a protobuf issue preventing us from doing that; see for example the failures on input-methods-can-... when I bumped protobuf ABI (eg: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5597/console )

The issue is that protobuf will abort() if it's loaded twice, and bumping mirprotobuf ABI ensures that it's loaded twice. Otherwise we wouldn't be building libmirprotobuf.so at all, and would be statically linking it as nature intended.

[Chris Halse Rogers (raof)]

So, urgh. Yeah, this is better than what we've got.

Is there any obvious way to test that we don't stack-create protobuf objects? It'd be nice to have a test, but I can't think of one.

review: Approve

[Alan Griffiths (alan-griffiths)]

> So I think Disapprove/Needs fixing. A simple bump of MIRPROTOBUF_ABI will
> solve it more reliably and give us more efficient code. Unless anyone can
> prove there's a protobuf issue preventing us from doing that.

It really isn't that simple: protobuf has a number of issues in this area. Essentially, we can only have one protobuf instantiation in process.

We've "got away" with it while the client ABI has remained at 8 (as only the latest limbirprotobuf gets loaded). Now we're proposing 9 we need to keep the same limbirprotobuf version.

Alexandros has tried to upstream some fixes to protobuf, and Kevin and Chris have recently run into problems.

AFAICS the approach proposed here is the least bad of the feasible options.

review: Approve

[Alberto Aguirre (albaguirre)]

@Daniel,

As others have said, yes there are protobuf issues preventing loading two versions in the same process (for more comments see Chris branch https://code.launchpad.net/~raof/mir/input-methods-can-specify-foreign-parents/+merge/258001)

Loading two versions of protobuf in the same process is more common than is apparent at first glance, because the mesa client platform module links against libmirclient.

Indeed, just by probing the modules, a client that links against libmirclient8, will end up loading libmirprotobuf.so.1 since the module will load libmirclient9 (which is linked to libmirprotobuf.so.1)

[Alberto Aguirre (albaguirre)]

"The solution presented here however fails to work in all cases of ABI breaks such as when a member is removed from the middle of the object. Not likely, but there is a better option..."

This is true, we still need to tread carefully when modifying protobuf message definitions.

=== modified file 'src/client/buffer_stream.cpp'
--- src/client/buffer_stream.cpp	2015-06-03 19:17:55 +0000
+++ src/client/buffer_stream.cpp	2015-06-16 23:39:42 +0000
@@ -19,6 +19,7 @@
 #define MIR_LOG_COMPONENT "MirBufferStream"
 
 #include "buffer_stream.h"
+#include "make_protobuf_object.h"
 
 #include "mir_connection.h"
 #include "mir/frontend/client_constants.h"
@@ -97,11 +98,11 @@
       display_server(server),
       mode(mode),
       client_platform(client_platform),
-      protobuf_bs(protobuf_bs),
+      protobuf_bs{mcl::make_protobuf_object<mir::protobuf::BufferStream>(protobuf_bs)},
       buffer_depository{client_platform->create_buffer_factory(), mir::frontend::client_buffer_cache_size},
       swap_interval_(1),
-      perf_report(perf_report)
-      
+      perf_report(perf_report),
+      protobuf_void{mcl::make_protobuf_object<mir::protobuf::Void>()}
 {
     created(nullptr, nullptr);
     perf_report->name_surface(surface_name.c_str());
@@ -119,21 +120,23 @@
       display_server(server),
       mode(BufferStreamMode::Producer),
       client_platform(client_platform),
+      protobuf_bs{mcl::make_protobuf_object<mir::protobuf::BufferStream>()},
       buffer_depository{client_platform->create_buffer_factory(), mir::frontend::client_buffer_cache_size},
       swap_interval_(1),
-      perf_report(perf_report)
+      perf_report(perf_report),
+      protobuf_void{mcl::make_protobuf_object<mir::protobuf::Void>()}
 {
     perf_report->name_surface(std::to_string(reinterpret_cast<long int>(this)).c_str());
 
     create_wait_handle.expect_result();
     try
     {
-        server.create_buffer_stream(0, &parameters, &protobuf_bs, gp::NewCallback(this, &mcl::BufferStream::created, callback,
+        server.create_buffer_stream(0, &parameters, protobuf_bs.get(), gp::NewCallback(this, &mcl::BufferStream::created, callback,
             context));
     }
     catch (std::exception const& ex)
     {
-        protobuf_bs.set_error(std::string{"Error invoking create buffer stream: "} +
+        protobuf_bs->set_error(std::string{"Error invoking create buffer stream: "} +
                               boost::diagnostic_information(ex));
     }
         
@@ -141,16 +144,16 @@
 
 void mcl::BufferStream::created(mir_buffer_stream_callback callback, void *context)
 {
-    if (!protobuf_bs.has_id() || protobuf_bs.has_error())
-        BOOST_THROW_EXCEPTION(std::runtime_error("Can not create buffer stream: " + std::string(protobuf_bs.error())));
-    if (!protobuf_bs.has_buffer())
+    if (!protobuf_bs->has_id() || protobuf_bs->has_error())
+        BOOST_THROW_EXCEPTION(std::runtime_error("Can not create buffer stream: " + std::string(protobuf_bs->error())));
+    if (!protobuf_bs->has_buffer())
         BOOST_THROW_EXCEPTION(std::runtime_error("Buffer stream did not come with a buffer"));
 
-    process_buffer(protobuf_bs.buffer());
+    process_buffer(protobuf_bs->buffer());
     egl_native_window_ = client_platform->create_egl_native_window(this);
 
     if (connection)
-        connection->on_stream_created(protobuf_bs.id().value(), this);
+        connection->on_stream_created(protobuf_bs->id().value(), this);
 
     if (callback)
         callback(reinterpret_cast<MirBufferStream*>(this), context);
@@ -180,7 +183,7 @@
 
     try
     {
-        auto pixel_format = static_cast<MirPixelFormat>(protobuf_bs.pixel_format());
+        auto pixel_format = static_cast<MirPixelFormat>(protobuf_bs->pixel_format());
         buffer_depository.deposit_package(buffer_package,
             buffer.buffer_id(),
             cached_buffer_size, pixel_format);
@@ -199,40 +202,37 @@
 
     secured_region.reset();
 
-    mir::protobuf::BufferStreamId buffer_stream_id;
-    buffer_stream_id.set_value(protobuf_bs.id().value());
-
-// TODO: We can fix the strange "ID casting" used below in the second phase
-// of buffer stream which generalizes and clarifies the server side logic.
+    // TODO: We can fix the strange "ID casting" used below in the second phase
+    // of buffer stream which generalizes and clarifies the server side logic.
     if (mode == mcl::BufferStreamMode::Producer)
     {
-        mp::BufferRequest request;
-        request.mutable_id()->set_value(protobuf_bs.id().value());
-        request.mutable_buffer()->set_buffer_id(protobuf_bs.buffer().buffer_id());
+        auto request = mcl::make_protobuf_object<mp::BufferRequest>();
+        request->mutable_id()->set_value(protobuf_bs->id().value());
+        request->mutable_buffer()->set_buffer_id(protobuf_bs->buffer().buffer_id());
 
         lock.unlock();
         next_buffer_wait_handle.expect_result();
 
         display_server.exchange_buffer(
             nullptr,
-            &request,
-            protobuf_bs.mutable_buffer(),
+            request.get(),
+            protobuf_bs->mutable_buffer(),
             google::protobuf::NewCallback(
             this, &mcl::BufferStream::next_buffer_received,
             done));
     }
     else
     {
-        mp::ScreencastId screencast_id;
-        screencast_id.set_value(protobuf_bs.id().value());
+        auto screencast_id = mcl::make_protobuf_object<mp::ScreencastId>();
+        screencast_id->set_value(protobuf_bs->id().value());
 
         lock.unlock();
         next_buffer_wait_handle.expect_result();
 
         display_server.screencast_buffer(
             nullptr,
-            &screencast_id,
-            protobuf_bs.mutable_buffer(),
+            screencast_id.get(),
+            protobuf_bs->mutable_buffer(),
             google::protobuf::NewCallback(
             this, &mcl::BufferStream::next_buffer_received,
             done));
@@ -270,7 +270,7 @@
 void mcl::BufferStream::next_buffer_received(
     std::function<void()> done)                                             
 {
-    process_buffer(protobuf_bs.buffer());
+    process_buffer(protobuf_bs->buffer());
 
     done();
 
@@ -285,8 +285,8 @@
         "",
         cached_buffer_size.width.as_int(),
         cached_buffer_size.height.as_int(),
-        static_cast<MirPixelFormat>(protobuf_bs.pixel_format()),
-        static_cast<MirBufferUsage>(protobuf_bs.buffer_usage()),
+        static_cast<MirPixelFormat>(protobuf_bs->pixel_format()),
+        static_cast<MirBufferUsage>(protobuf_bs->buffer_usage()),
         mir_display_output_id_invalid};
 }
 
@@ -318,20 +318,21 @@
         BOOST_THROW_EXCEPTION(std::logic_error("Attempt to set swap interval on screencast is invalid"));
     }
 
-    mp::SurfaceSetting setting, result;
-    setting.mutable_surfaceid()->set_value(protobuf_bs.id().value());
-    setting.set_attrib(attrib);
-    setting.set_ivalue(value);
+    auto setting = mcl::make_protobuf_object<mp::SurfaceSetting>();
+    auto result = mcl::make_protobuf_object<mp::SurfaceSetting>();
+    setting->mutable_surfaceid()->set_value(protobuf_bs->id().value());
+    setting->set_attrib(attrib);
+    setting->set_ivalue(value);
     lock.unlock();
 
     configure_wait_handle.expect_result();
-    display_server.configure_surface(0, &setting, &result,
+    display_server.configure_surface(0, setting.get(), result.get(),
         google::protobuf::NewCallback(this, &mcl::BufferStream::on_configured));
 
     configure_wait_handle.wait_for_all();
 
     lock.lock();
-    swap_interval_ = result.ivalue();
+    swap_interval_ = result->ivalue();
 }
 
 uint32_t mcl::BufferStream::get_current_buffer_id()
@@ -381,13 +382,13 @@
 {
     std::unique_lock<decltype(mutex)> lock(mutex);
     
-    return mf::BufferStreamId(protobuf_bs.id().value());
+    return mf::BufferStreamId(protobuf_bs->id().value());
 }
 
 bool mcl::BufferStream::valid() const
 {
     std::unique_lock<decltype(mutex)> lock(mutex);
-    return protobuf_bs.has_id() && !protobuf_bs.has_error();
+    return protobuf_bs->has_id() && !protobuf_bs->has_error();
 }
 
 void mcl::BufferStream::set_buffer_cache_size(unsigned int cache_size)

=== modified file 'src/client/buffer_stream.h'
--- src/client/buffer_stream.h	2015-06-03 19:17:55 +0000
+++ src/client/buffer_stream.h	2015-06-16 23:39:42 +0000
@@ -62,7 +62,7 @@
         mir::protobuf::DisplayServer& server,
         BufferStreamMode mode,
         std::shared_ptr<ClientPlatform> const& native_window_factory,
-        protobuf::BufferStream const& protobuf_bs,
+        mir::protobuf::BufferStream const& protobuf_bs,
         std::shared_ptr<PerfReport> const& perf_report,
         std::string const& surface_name);
     // For surfaceless buffer streams
@@ -124,7 +124,7 @@
     BufferStreamMode const mode;
     std::shared_ptr<ClientPlatform> const client_platform;
 
-    mir::protobuf::BufferStream protobuf_bs;
+    std::unique_ptr<mir::protobuf::BufferStream> protobuf_bs;
     mir::client::ClientBufferDepository buffer_depository;
     
     int swap_interval_;
@@ -137,7 +137,7 @@
     MirWaitHandle release_wait_handle;
     MirWaitHandle next_buffer_wait_handle;
     MirWaitHandle configure_wait_handle;
-    mir::protobuf::Void protobuf_void;
+    std::unique_ptr<mir::protobuf::Void> protobuf_void;
     
     std::shared_ptr<MemoryRegion> secured_region;
     

=== added file 'src/client/make_protobuf_object.h'
--- src/client/make_protobuf_object.h	1970-01-01 00:00:00 +0000
+++ src/client/make_protobuf_object.h	2015-06-16 23:39:42 +0000
@@ -0,0 +1,42 @@
+/*
+ * Copyright © 2015 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser 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 Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Alberto Aguirre <[e-mail]>
+ */
+
+#ifndef MIR_CLIENT_MAKE_PROTOBUF_OBJECT_
+#define MIR_CLIENT_MAKE_PROTOBUF_OBJECT_
+
+namespace mir
+{
+namespace client
+{
+template <typename ProtobufType>
+auto make_protobuf_object()
+{
+    return std::unique_ptr<ProtobufType>{ProtobufType::default_instance().New()};
+}
+
+template <typename ProtobufType>
+auto make_protobuf_object(ProtobufType const& from)
+{
+    auto object = make_protobuf_object<ProtobufType>();
+    object->CopyFrom(from);
+    return object;
+}
+}
+}
+
+#endif

=== modified file 'src/client/mir_connection.cpp'
--- src/client/mir_connection.cpp	2015-06-03 17:01:43 +0000
+++ src/client/mir_connection.cpp	2015-06-16 23:39:42 +0000
@@ -20,6 +20,7 @@
 #include "mir_surface.h"
 #include "mir_prompt_session.h"
 #include "default_client_buffer_stream_factory.h"
+#include "make_protobuf_object.h"
 #include "mir_toolkit/mir_platform_message.h"
 #include "mir/client_platform.h"
 #include "mir/client_platform_factory.h"
@@ -107,7 +108,13 @@
         server(channel.get(), ::google::protobuf::Service::STUB_DOESNT_OWN_CHANNEL),
         debug(channel.get(), ::google::protobuf::Service::STUB_DOESNT_OWN_CHANNEL),
         logger(conf.the_logger()),
+        void_response{mcl::make_protobuf_object<mir::protobuf::Void>()},
+        connect_result{mcl::make_protobuf_object<mir::protobuf::Connection>()},
         connect_done{false},
+        ignored{mcl::make_protobuf_object<mir::protobuf::Void>()},
+        connect_parameters{mcl::make_protobuf_object<mir::protobuf::ConnectParameters>()},
+        platform_operation_reply{mcl::make_protobuf_object<mir::protobuf::PlatformOperationMessage>()},
+        display_configuration_response{mcl::make_protobuf_object<mir::protobuf::DisplayConfiguration>()},
         client_platform_factory(conf.the_client_platform_factory()),
         input_platform(conf.the_input_platform()),
         display_configuration(conf.the_display_configuration()),
@@ -116,7 +123,7 @@
         event_handler_register(conf.the_event_handler_register()),
         eventloop{new md::ThreadedDispatcher{"RPC Thread", std::dynamic_pointer_cast<md::Dispatchable>(channel)}}
 {
-    connect_result.set_error("connect not called");
+    connect_result->set_error("connect not called");
     {
         std::lock_guard<std::mutex> lock(connection_guard);
         next_valid = valid_connections;
@@ -131,9 +138,9 @@
     connect_wait_handle.wait_for_pending(std::chrono::milliseconds(500));
 
     std::lock_guard<decltype(mutex)> lock(mutex);
-    if (connect_result.has_platform())
+    if (connect_result && connect_result->has_platform())
     {
-        auto const& platform = connect_result.platform();
+        auto const& platform = connect_result->platform();
         for (int i = 0, end = platform.fd_size(); i != end; ++i)
             close(platform.fd(i));
     }
@@ -154,9 +161,9 @@
 {
     std::lock_guard<decltype(mutex)> lock(mutex);
 
-    if (connect_result.has_error())
+    if (connect_result && connect_result->has_error())
     {
-        return connect_result.error().c_str();
+        return connect_result->error().c_str();
     }
 
     return error_message.c_str();
@@ -220,8 +227,8 @@
 
     SurfaceRelease surf_release{surface, new_wait_handle, callback, context};
 
-    mir::protobuf::SurfaceId message;
-    message.set_value(surface->id());
+    auto message = mcl::make_protobuf_object<mir::protobuf::SurfaceId>();
+    message->set_value(surface->id());
 
     {
         std::lock_guard<decltype(release_wait_handle_guard)> rel_lock(release_wait_handle_guard);
@@ -229,7 +236,7 @@
     }
 
     new_wait_handle->expect_result();
-    server.release_surface(0, &message, &void_response,
+    server.release_surface(0, message.get(), void_response.get(),
                            gp::NewCallback(this, &MirConnection::released, surf_release));
 
 
@@ -248,9 +255,9 @@
     {
         std::lock_guard<decltype(mutex)> lock(mutex);
 
-        if (!connect_result.has_platform() || !connect_result.has_display_configuration())
+        if (!connect_result->has_platform() || !connect_result->has_display_configuration())
         {
-            if (!connect_result.has_error())
+            if (!connect_result->has_error())
             {
                 // We're handling an error scenario that means we're not sync'd
                 // with the client code - a callback isn't safe (or needed)
@@ -283,12 +290,12 @@
         buffer_stream_factory = std::make_shared<mcl::DefaultClientBufferStreamFactory>(
             platform, the_logger());
         native_display = platform->create_egl_native_display();
-        display_configuration->set_configuration(connect_result.display_configuration());
+        display_configuration->set_configuration(connect_result->display_configuration());
         lifecycle_control->set_lifecycle_event_handler(default_lifecycle_event_handler);
     }
     catch (std::exception const& e)
     {
-        connect_result.set_error(std::string{"Failed to process connect response: "} +
+        connect_result->set_error(std::string{"Failed to process connect response: "} +
                                  boost::diagnostic_information(e));
     }
 
@@ -304,14 +311,14 @@
     {
         std::lock_guard<decltype(mutex)> lock(mutex);
 
-        connect_parameters.set_application_name(app_name);
+        connect_parameters->set_application_name(app_name);
         connect_wait_handle.expect_result();
     }
 
     server.connect(
         0,
-        &connect_parameters,
-        &connect_result,
+        connect_parameters.get(),
+        connect_result.get(),
         google::protobuf::NewCallback(
             this, &MirConnection::connected, callback, context));
     return &connect_wait_handle;
@@ -339,7 +346,7 @@
         disconnecting = true;
     }
     disconnect_wait_handle.expect_result();
-    server.disconnect(0, &ignored, &ignored,
+    server.disconnect(0, ignored.get(), ignored.get(),
                       google::protobuf::NewCallback(this, &MirConnection::done_disconnect));
 
     return &disconnect_wait_handle;
@@ -348,19 +355,19 @@
 void MirConnection::done_platform_operation(
     mir_platform_operation_callback callback, void* context)
 {
-    auto reply = mir_platform_message_create(platform_operation_reply.opcode());
+    auto reply = mir_platform_message_create(platform_operation_reply->opcode());
 
-    set_error_message(platform_operation_reply.error());
+    set_error_message(platform_operation_reply->error());
 
     mir_platform_message_set_data(
         reply,
-        platform_operation_reply.data().data(),
-        platform_operation_reply.data().size());
+        platform_operation_reply->data().data(),
+        platform_operation_reply->data().size());
 
     mir_platform_message_set_fds(
         reply,
-        platform_operation_reply.fd().data(),
-        platform_operation_reply.fd().size());
+        platform_operation_reply->fd().data(),
+        platform_operation_reply->fd().size());
 
     callback(this, reply, context);
 
@@ -379,21 +386,21 @@
         return nullptr;
     }
 
-    mir::protobuf::PlatformOperationMessage protobuf_request;
+    auto protobuf_request = mcl::make_protobuf_object<mir::protobuf::PlatformOperationMessage>();
 
-    protobuf_request.set_opcode(mir_platform_message_get_opcode(request));
+    protobuf_request->set_opcode(mir_platform_message_get_opcode(request));
     auto const request_data = mir_platform_message_get_data(request);
     auto const request_fds = mir_platform_message_get_fds(request);
 
-    protobuf_request.set_data(request_data.data, request_data.size);
+    protobuf_request->set_data(request_data.data, request_data.size);
     for (size_t i = 0; i != request_fds.num_fds; ++i)
-        protobuf_request.add_fd(request_fds.fds[i]);
+        protobuf_request->add_fd(request_fds.fds[i]);
 
     platform_operation_wait_handle.expect_result();
     server.platform_operation(
         0,
-        &protobuf_request,
-        &platform_operation_reply,
+        protobuf_request.get(),
+        platform_operation_reply.get(),
         google::protobuf::NewCallback(this, &MirConnection::done_platform_operation,
                                       callback, context));
 
@@ -411,7 +418,7 @@
             {
                 lock.unlock();
                 std::lock_guard<decltype(connection->mutex)> lock(connection->mutex);
-                return !connection->connect_result.has_error();
+                return !connection->connect_result->has_error();
             }
         }
     }
@@ -427,9 +434,9 @@
 {
     // connect_result is write-once: once it's valid, we don't need to lock
     // to use it.
-    if (connect_done && !connect_result.has_error() && connect_result.has_platform())
+    if (connect_done && !connect_result->has_error() && connect_result->has_platform())
     {
-        auto const& platform = connect_result.platform();
+        auto const& platform = connect_result->platform();
 
         platform_package.data_items = platform.data_size();
         for (int i = 0; i != platform.data_size(); ++i)
@@ -460,15 +467,15 @@
     valid_formats = 0;
 
     std::lock_guard<decltype(mutex)> lock(mutex);
-    if (!connect_result.has_error())
+    if (!connect_result->has_error())
     {
         valid_formats = std::min(
-            static_cast<unsigned int>(connect_result.surface_pixel_format_size()),
+            static_cast<unsigned int>(connect_result->surface_pixel_format_size()),
             formats_size);
 
         for (auto i = 0u; i < valid_formats; i++)
         {
-            formats[i] = static_cast<MirPixelFormat>(connect_result.surface_pixel_format(i));
+            formats[i] = static_cast<MirPixelFormat>(connect_result->surface_pixel_format(i));
         }
     }
 }
@@ -488,13 +495,13 @@
     mir_buffer_stream_callback callback,
     void *context)
 {
-    mir::protobuf::BufferStreamParameters params;
-    params.set_width(width);
-    params.set_height(height);
-    params.set_pixel_format(format);
-    params.set_buffer_usage(buffer_usage);
+    auto params = mcl::make_protobuf_object<mir::protobuf::BufferStreamParameters>();
+    params->set_width(width);
+    params->set_height(height);
+    params->set_pixel_format(format);
+    params->set_buffer_usage(buffer_usage);
 
-    return buffer_stream_factory->make_producer_stream(this, server, params, callback, context);
+    return buffer_stream_factory->make_producer_stream(this, server, *params, callback, context);
 }
 
 std::shared_ptr<mir::client::ClientBufferStream> MirConnection::make_consumer_stream(
@@ -559,10 +566,10 @@
 {
     std::lock_guard<decltype(mutex)> lock(mutex);
 
-    set_error_message(display_configuration_response.error());
+    set_error_message(display_configuration_response->error());
 
-    if (!display_configuration_response.has_error())
-        display_configuration->set_configuration(display_configuration_response);
+    if (!display_configuration_response->has_error())
+        display_configuration->set_configuration(*display_configuration_response);
 
     return configure_display_wait_handle.result_received();
 }
@@ -574,14 +581,14 @@
         return NULL;
     }
 
-    mir::protobuf::DisplayConfiguration request;
+    auto request = mcl::make_protobuf_object<mir::protobuf::DisplayConfiguration>();
     {
         std::lock_guard<decltype(mutex)> lock(mutex);
 
         for (auto i=0u; i < config->num_outputs; i++)
         {
             auto output = config->outputs[i];
-            auto display_request = request.add_display_output();
+            auto display_request = request->add_display_output();
             display_request->set_output_id(output.output_id);
             display_request->set_used(output.used);
             display_request->set_current_mode(output.current_mode);
@@ -594,7 +601,7 @@
     }
 
     configure_display_wait_handle.expect_result();
-    server.configure_display(0, &request, &display_configuration_response,
+    server.configure_display(0, request.get(), display_configuration_response.get(),
         google::protobuf::NewCallback(this, &MirConnection::done_display_configure));
 
     return &configure_display_wait_handle;
@@ -619,8 +626,8 @@
 
     StreamRelease stream_release{stream, new_wait_handle, callback, context};
 
-    mir::protobuf::BufferStreamId buffer_stream_id;
-    buffer_stream_id.set_value(stream->rpc_id().as_value());
+    auto buffer_stream_id = mcl::make_protobuf_object<mir::protobuf::BufferStreamId>();
+    buffer_stream_id->set_value(stream->rpc_id().as_value());
 
     {
         std::lock_guard<decltype(release_wait_handle_guard)> rel_lock(release_wait_handle_guard);
@@ -629,7 +636,7 @@
 
     new_wait_handle->expect_result();
     server.release_buffer_stream(
-        nullptr, &buffer_stream_id, &void_response,
+        nullptr, buffer_stream_id.get(), void_response.get(),
         google::protobuf::NewCallback(this, &MirConnection::released, stream_release));
     return new_wait_handle;
 }

=== modified file 'src/client/mir_connection.h'
--- src/client/mir_connection.h	2015-06-03 17:01:43 +0000
+++ src/client/mir_connection.h	2015-06-16 23:39:42 +0000
@@ -35,6 +35,8 @@
 
 #include "mir_wait_handle.h"
 
+#include <memory>
+
 namespace mir
 {
 class SharedLibrary;
@@ -172,13 +174,13 @@
     mir::protobuf::DisplayServer::Stub server;
     mir::protobuf::Debug::Stub debug;
     std::shared_ptr<mir::logging::Logger> const logger;
-    mir::protobuf::Void void_response;
-    mir::protobuf::Connection connect_result;
+    std::unique_ptr<mir::protobuf::Void> void_response;
+    std::unique_ptr<mir::protobuf::Connection> connect_result;
     std::atomic<bool> connect_done;
-    mir::protobuf::Void ignored;
-    mir::protobuf::ConnectParameters connect_parameters;
-    mir::protobuf::PlatformOperationMessage platform_operation_reply;
-    mir::protobuf::DisplayConfiguration display_configuration_response;
+    std::unique_ptr<mir::protobuf::Void> ignored;
+    std::unique_ptr<mir::protobuf::ConnectParameters> connect_parameters;
+    std::unique_ptr<mir::protobuf::PlatformOperationMessage> platform_operation_reply;
+    std::unique_ptr<mir::protobuf::DisplayConfiguration> display_configuration_response;
     std::atomic<bool> disconnecting{false};
 
     std::shared_ptr<mir::client::ClientPlatformFactory> const client_platform_factory;

=== modified file 'src/client/mir_prompt_session.cpp'
--- src/client/mir_prompt_session.cpp	2015-03-31 02:35:42 +0000
+++ src/client/mir_prompt_session.cpp	2015-06-16 23:39:42 +0000
@@ -18,6 +18,7 @@
 
 #include "mir_prompt_session.h"
 #include "event_handler_register.h"
+#include "make_protobuf_object.h"
 
 namespace mp = mir::protobuf;
 namespace mcl = mir::client;
@@ -26,6 +27,10 @@
     mp::DisplayServer& server,
     std::shared_ptr<mcl::EventHandlerRegister> const& event_handler_register) :
     server(server),
+    parameters(mcl::make_protobuf_object<mir::protobuf::PromptSessionParameters>()),
+    add_result(mcl::make_protobuf_object<mir::protobuf::Void>()),
+    protobuf_void(mcl::make_protobuf_object<mir::protobuf::Void>()),
+    socket_fd_response(mcl::make_protobuf_object<mir::protobuf::SocketFD>()),
     event_handler_register(event_handler_register),
     event_handler_register_id{event_handler_register->register_event_handler(
         [this](MirEvent const& event)
@@ -34,6 +39,7 @@
                 set_state(mir_prompt_session_event_get_state(mir_event_get_prompt_session_event(&event)));
         })},
     state(mir_prompt_session_state_stopped),
+    session(mcl::make_protobuf_object<mir::protobuf::Void>()),
     handle_prompt_session_state_change{[](MirPromptSessionState){}}
 {
 }
@@ -64,14 +70,14 @@
 {
     {
         std::lock_guard<decltype(mutex)> lock(mutex);
-        parameters.set_application_pid(application_pid);
+        parameters->set_application_pid(application_pid);
     }
 
     start_wait_handle.expect_result();
     server.start_prompt_session(
         0,
-        &parameters,
-        &session,
+        parameters.get(),
+        session.get(),
         google::protobuf::NewCallback(this, &MirPromptSession::done_start,
                                       callback, context));
 
@@ -84,8 +90,8 @@
 
     server.stop_prompt_session(
         0,
-        &protobuf_void,
-        &protobuf_void,
+        protobuf_void.get(),
+        protobuf_void.get(),
         google::protobuf::NewCallback(this, &MirPromptSession::done_stop,
                                       callback, context));
 
@@ -110,7 +116,7 @@
     {
         std::lock_guard<decltype(session_mutex)> lock(session_mutex);
 
-        state = session.has_error() ? mir_prompt_session_state_stopped : mir_prompt_session_state_started;
+        state = session->has_error() ? mir_prompt_session_state_stopped : mir_prompt_session_state_started;
     }
 
     callback(this, context);
@@ -129,10 +135,10 @@
 {
     std::lock_guard<decltype(session_mutex)> lock(session_mutex);
 
-    if (!session.has_error())
-        session.set_error(std::string{});
+    if (!session->has_error())
+        session->set_error(std::string{});
 
-    return session.error().c_str();
+    return session->error().c_str();
 }
 
 MirWaitHandle* MirPromptSession::new_fds_for_prompt_providers(
@@ -140,15 +146,15 @@
     mir_client_fd_callback callback,
     void * context)
 {
-    mir::protobuf::SocketFDRequest request;
-    request.set_number(no_of_fds);
+    auto request = mcl::make_protobuf_object<mir::protobuf::SocketFDRequest>();
+    request->set_number(no_of_fds);
 
     fds_for_prompt_providers_wait_handle.expect_result();
 
     server.new_fds_for_prompt_providers(
         nullptr,
-        &request,
-        &socket_fd_response,
+        request.get(),
+        socket_fd_response.get(),
         google::protobuf::NewCallback(this, &MirPromptSession::done_fds_for_prompt_providers,
                                               callback, context));
 
@@ -159,13 +165,13 @@
     mir_client_fd_callback callback,
     void* context)
 {
-    auto const size = socket_fd_response.fd_size();
+    auto const size = socket_fd_response->fd_size();
 
     std::vector<int> fds;
     fds.reserve(size);
 
     for (auto i = 0; i != size; ++i)
-        fds.push_back(socket_fd_response.fd(i));
+        fds.push_back(socket_fd_response->fd(i));
 
     callback(this, size, fds.data(), context);
     fds_for_prompt_providers_wait_handle.result_received();

=== modified file 'src/client/mir_prompt_session.h'
--- src/client/mir_prompt_session.h	2015-01-21 07:34:50 +0000
+++ src/client/mir_prompt_session.h	2015-06-16 23:39:42 +0000
@@ -60,10 +60,10 @@
 private:
     std::mutex mutable mutex; // Protects parameters, wait_handles & results
     mir::protobuf::DisplayServer& server;
-    mir::protobuf::PromptSessionParameters parameters;
-    mir::protobuf::Void add_result;
-    mir::protobuf::Void protobuf_void;
-    mir::protobuf::SocketFD socket_fd_response;
+    std::unique_ptr<mir::protobuf::PromptSessionParameters> parameters;
+    std::unique_ptr<mir::protobuf::Void> add_result;
+    std::unique_ptr<mir::protobuf::Void> protobuf_void;
+    std::unique_ptr<mir::protobuf::SocketFD> socket_fd_response;
     std::shared_ptr<mir::client::EventHandlerRegister> const event_handler_register;
     int const event_handler_register_id;
 
@@ -73,7 +73,7 @@
     std::atomic<MirPromptSessionState> state;
 
     std::mutex mutable session_mutex; // Protects session
-    mir::protobuf::Void session;
+    std::unique_ptr<mir::protobuf::Void> session;
 
     std::mutex mutable event_handler_mutex; // Need another mutex for callback access to members
     std::function<void(MirPromptSessionState)> handle_prompt_session_state_change;

=== modified file 'src/client/mir_screencast.cpp'
--- src/client/mir_screencast.cpp	2015-06-03 17:35:29 +0000
+++ src/client/mir_screencast.cpp	2015-06-16 23:39:42 +0000
@@ -18,6 +18,7 @@
 
 #include "mir_screencast.h"
 #include "mir_connection.h"
+#include "make_protobuf_object.h"
 #include "client_buffer_stream.h"
 #include "mir/frontend/client_constants.h"
 #include "mir_toolkit/mir_native_buffer.h"
@@ -38,7 +39,9 @@
     mir_screencast_callback callback, void* context)
     : server(server),
       connection{connection},
-      output_size{size}
+      output_size{size},
+      protobuf_screencast{mcl::make_protobuf_object<mir::protobuf::Screencast>()},
+      protobuf_void{mcl::make_protobuf_object<mir::protobuf::Void>()}
 {
     if (output_size.width.as_int()  == 0 ||
         output_size.height.as_int() == 0 ||
@@ -48,23 +51,23 @@
     {
         BOOST_THROW_EXCEPTION(std::runtime_error("Invalid parameters"));
     }
-    protobuf_screencast.set_error("Not initialized");
-
-    mir::protobuf::ScreencastParameters parameters;
-
-    parameters.mutable_region()->set_left(region.top_left.x.as_int());
-    parameters.mutable_region()->set_top(region.top_left.y.as_int());
-    parameters.mutable_region()->set_width(region.size.width.as_uint32_t());
-    parameters.mutable_region()->set_height(region.size.height.as_uint32_t());
-    parameters.set_width(output_size.width.as_uint32_t());
-    parameters.set_height(output_size.height.as_uint32_t());
-    parameters.set_pixel_format(pixel_format);
+    protobuf_screencast->set_error("Not initialized");
+
+    auto parameters = mcl::make_protobuf_object<mir::protobuf::ScreencastParameters>();
+
+    parameters->mutable_region()->set_left(region.top_left.x.as_int());
+    parameters->mutable_region()->set_top(region.top_left.y.as_int());
+    parameters->mutable_region()->set_width(region.size.width.as_uint32_t());
+    parameters->mutable_region()->set_height(region.size.height.as_uint32_t());
+    parameters->set_width(output_size.width.as_uint32_t());
+    parameters->set_height(output_size.height.as_uint32_t());
+    parameters->set_pixel_format(pixel_format);
 
     create_screencast_wait_handle.expect_result();
     server.create_screencast(
         nullptr,
-        &parameters,
-        &protobuf_screencast,
+        parameters.get(),
+        protobuf_screencast.get(),
         google::protobuf::NewCallback(
             this, &MirScreencast::screencast_created,
             callback, context));
@@ -77,20 +80,20 @@
 
 bool MirScreencast::valid()
 {
-    return !protobuf_screencast.has_error();
+    return !protobuf_screencast->has_error();
 }
 
 MirWaitHandle* MirScreencast::release(
         mir_screencast_callback callback, void* context)
 {
-    mir::protobuf::ScreencastId screencast_id;
-    screencast_id.set_value(protobuf_screencast.screencast_id().value());
+    auto screencast_id = mcl::make_protobuf_object<mir::protobuf::ScreencastId>();
+    screencast_id->set_value(protobuf_screencast->screencast_id().value());
     
     release_wait_handle.expect_result();
     server.release_screencast(
         nullptr,
-        &screencast_id,
-        &protobuf_void,
+        screencast_id.get(),
+        protobuf_void.get(),
         google::protobuf::NewCallback(
             this, &MirScreencast::released, callback, context));
 
@@ -104,10 +107,10 @@
 void MirScreencast::screencast_created(
     mir_screencast_callback callback, void* context)
 {
-    if (!protobuf_screencast.has_error() && connection)
+    if (!protobuf_screencast->has_error() && connection)
     {
         buffer_stream = connection->make_consumer_stream(
-            protobuf_screencast.buffer_stream(), "MirScreencast");
+            protobuf_screencast->buffer_stream(), "MirScreencast");
     }
 
     callback(this, context);

=== modified file 'src/client/mir_screencast.h'
--- src/client/mir_screencast.h	2015-06-03 17:35:29 +0000
+++ src/client/mir_screencast.h	2015-06-16 23:39:42 +0000
@@ -27,6 +27,8 @@
 
 #include <EGL/eglplatform.h>
 
+#include <memory>
+
 namespace mir
 {
 namespace protobuf { class DisplayServer; }
@@ -71,8 +73,8 @@
     mir::geometry::Size const output_size;
     std::shared_ptr<mir::client::ClientBufferStream> buffer_stream;
 
-    mir::protobuf::Screencast protobuf_screencast;
-    mir::protobuf::Void protobuf_void;
+    std::unique_ptr<mir::protobuf::Screencast> protobuf_screencast;
+    std::unique_ptr<mir::protobuf::Void> protobuf_void;
 
     MirWaitHandle create_screencast_wait_handle;
     MirWaitHandle release_wait_handle;

=== modified file 'src/client/mir_surface.cpp'
--- src/client/mir_surface.cpp	2015-06-16 12:18:54 +0000
+++ src/client/mir_surface.cpp	2015-06-16 23:39:42 +0000
@@ -16,12 +16,13 @@
  * Authored by: Thomas Guest <[e-mail]>
  */
 
+#include "mir_surface.h"
+#include "cursor_configuration.h"
+#include "client_buffer_stream_factory.h"
+#include "make_protobuf_object.h"
 #include "mir_toolkit/mir_client_library.h"
 #include "mir/frontend/client_constants.h"
 #include "mir/client_buffer.h"
-#include "mir_surface.h"
-#include "cursor_configuration.h"
-#include "client_buffer_stream_factory.h"
 #include "mir_connection.h"
 #include "client_buffer_stream.h"
 #include "mir/dispatch/threaded_dispatcher.h"
@@ -42,7 +43,7 @@
 
 #define SERIALIZE_OPTION_IF_SET(option, message) \
     if (option.is_set()) \
-        message.set_##option(option.value());
+        message->set_##option(option.value());
 
 namespace
 {
@@ -79,9 +80,10 @@
 {
 }
 
-mir::protobuf::SurfaceParameters MirSurfaceSpec::serialize() const
+std::unique_ptr<mir::protobuf::SurfaceParameters> MirSurfaceSpec::serialize() const
 {
-    mir::protobuf::SurfaceParameters message;
+    //std::unique_ptr<mp::SurfaceParameters> message{mp::SurfaceParameters::default_instance().New()};
+    auto message = mcl::make_protobuf_object<mp::SurfaceParameters>();
 
     SERIALIZE_OPTION_IF_SET(width, message);
     SERIALIZE_OPTION_IF_SET(height, message);
@@ -103,26 +105,26 @@
     // max_aspect is a special case (below)
 
     if (parent.is_set() && parent.value() != nullptr)
-        message.set_parent_id(parent.value()->id());
+        message->set_parent_id(parent.value()->id());
 
     if (aux_rect.is_set())
     {
-        message.mutable_aux_rect()->set_left(aux_rect.value().left);
-        message.mutable_aux_rect()->set_top(aux_rect.value().top);
-        message.mutable_aux_rect()->set_width(aux_rect.value().width);
-        message.mutable_aux_rect()->set_height(aux_rect.value().height);
+        message->mutable_aux_rect()->set_left(aux_rect.value().left);
+        message->mutable_aux_rect()->set_top(aux_rect.value().top);
+        message->mutable_aux_rect()->set_width(aux_rect.value().width);
+        message->mutable_aux_rect()->set_height(aux_rect.value().height);
     }
 
     if (min_aspect.is_set())
     {
-        message.mutable_min_aspect()->set_width(min_aspect.value().width);
-        message.mutable_min_aspect()->set_height(min_aspect.value().height);
+        message->mutable_min_aspect()->set_width(min_aspect.value().width);
+        message->mutable_min_aspect()->set_height(min_aspect.value().height);
     }
 
     if (max_aspect.is_set())
     {
-        message.mutable_max_aspect()->set_width(max_aspect.value().width);
-        message.mutable_max_aspect()->set_height(max_aspect.value().height);
+        message->mutable_max_aspect()->set_width(max_aspect.value().width);
+        message->mutable_max_aspect()->set_height(max_aspect.value().height);
     }
 
     return message;
@@ -139,8 +141,9 @@
 }
 
 MirSurface::MirSurface(std::string const& error)
+    : surface{mcl::make_protobuf_object<mir::protobuf::Surface>()}
 {
-    surface.set_error(error);
+    surface->set_error(error);
 
     std::lock_guard<decltype(handle_mutex)> lock(handle_mutex);
     valid_surfaces.insert(this);
@@ -156,11 +159,16 @@
     mir_surface_callback callback, void * context)
     : server{&the_server},
       debug{debug},
+      surface{mcl::make_protobuf_object<mir::protobuf::Surface>()},
+      persistent_id{mcl::make_protobuf_object<mir::protobuf::PersistentSurfaceId>()},
       name{spec.surface_name.is_set() ? spec.surface_name.value() : ""},
+      void_response{mcl::make_protobuf_object<mir::protobuf::Void>()},
+      modify_result{mcl::make_protobuf_object<mir::protobuf::Void>()},
       connection(allocating_connection),
       buffer_stream_factory(buffer_stream_factory),
       input_platform(input_platform),
-      keymapper(std::make_shared<mircv::XKBMapper>())
+      keymapper(std::make_shared<mircv::XKBMapper>()),
+      configure_result{mcl::make_protobuf_object<mir::protobuf::SurfaceSetting>()}
 {
     for (int i = 0; i < mir_surface_attribs; i++)
         attrib_cache[i] = -1;
@@ -169,11 +177,11 @@
     create_wait_handle.expect_result();
     try 
     {
-        server->create_surface(0, &message, &surface, gp::NewCallback(this, &MirSurface::created, callback, context));
+        server->create_surface(0, message.get(), surface.get(), gp::NewCallback(this, &MirSurface::created, callback, context));
     }
     catch (std::exception const& ex)
     {
-        surface.set_error(std::string{"Error invoking create surface: "} +
+        surface->set_error(std::string{"Error invoking create surface: "} +
                           boost::diagnostic_information(ex));
     }
 
@@ -192,8 +200,8 @@
 
     input_thread.reset();
 
-    for (auto i = 0, end = surface.fd_size(); i != end; ++i)
-        close(surface.fd(i));
+    for (auto i = 0, end = surface->fd_size(); i != end; ++i)
+        close(surface->fd(i));
 }
 
 MirSurfaceParameters MirSurface::get_parameters() const
@@ -207,9 +215,9 @@
 {
     std::lock_guard<decltype(mutex)> lock(mutex);
 
-    if (surface.has_error())
+    if (surface->has_error())
     {
-        return surface.error().c_str();
+        return surface->error().c_str();
     }
     return error_message.c_str();
 }
@@ -218,7 +226,7 @@
 {
     std::lock_guard<decltype(mutex)> lock(mutex);
 
-    return surface.id().value();
+    return surface->id().value();
 }
 
 bool MirSurface::is_valid(MirSurface* query)
@@ -226,16 +234,16 @@
     std::lock_guard<decltype(handle_mutex)> lock(handle_mutex);
 
     if (valid_surfaces.count(query))
-        return !query->surface.has_error();
+        return !query->surface->has_error();
 
     return false;
 }
 
 void MirSurface::acquired_persistent_id(mir_surface_id_callback callback, void* context)
 {
-    if (!persistent_id.has_error())
+    if (!persistent_id->has_error())
     {
-        callback(this, new MirPersistentId{persistent_id.value()}, context);
+        callback(this, new MirPersistentId{persistent_id->value()}, context);
     }
     else
     {
@@ -248,20 +256,20 @@
 {
     std::lock_guard<decltype(mutex)> lock{mutex};
 
-    if (persistent_id.has_value())
+    if (persistent_id->has_value())
     {
-        callback(this, new MirPersistentId{persistent_id.value()}, context);
+        callback(this, new MirPersistentId{persistent_id->value()}, context);
         return nullptr;
     }
 
     persistent_id_wait_handle.expect_result();
     try
     {
-        server->request_persistent_surface_id(0, &surface.id(), &persistent_id, gp::NewCallback(this, &MirSurface::acquired_persistent_id, callback, context));
+        server->request_persistent_surface_id(0, &surface->id(), persistent_id.get(), gp::NewCallback(this, &MirSurface::acquired_persistent_id, callback, context));
     }
     catch (std::exception const& ex)
     {
-        surface.set_error(std::string{"Failed to acquire a persistent ID from the server: "} +
+        surface->set_error(std::string{"Failed to acquire a persistent ID from the server: "} +
                           boost::diagnostic_information(ex));
     }
     return &persistent_id_wait_handle;
@@ -283,10 +291,10 @@
 {
     {
     std::lock_guard<decltype(mutex)> lock(mutex);
-    if (!surface.has_id())
+    if (!surface->has_id())
     {
-        if (!surface.has_error())
-            surface.set_error("Error processing surface create response, no ID (disconnected?)");
+        if (!surface->has_error())
+            surface->set_error("Error processing surface create response, no ID (disconnected?)");
 
         callback(this, context);
         create_wait_handle.result_received();
@@ -299,11 +307,11 @@
             std::lock_guard<decltype(mutex)> lock(mutex);
 
             buffer_stream = buffer_stream_factory->
-                make_producer_stream(connection, *server, surface.buffer_stream(), name);
+                make_producer_stream(connection, *server, surface->buffer_stream(), name);
 
-            for(int i = 0; i < surface.attributes_size(); i++)
+            for(int i = 0; i < surface->attributes_size(); i++)
             {
-                auto const& attrib = surface.attributes(i);
+                auto const& attrib = surface->attributes(i);
                 attrib_cache[attrib.attrib()] = attrib.ivalue();
             }
         }
@@ -312,7 +320,7 @@
     }
     catch (std::exception const& error)
     {
-        surface.set_error(std::string{"Error processing Surface creating response:"} +
+        surface->set_error(std::string{"Error processing Surface creating response:"} +
                           boost::diagnostic_information(error));
     }
 
@@ -331,7 +339,7 @@
             was_valid = true;
         valid_surfaces.erase(this);
     }
-    if (this->surface.has_error())
+    if (this->surface->has_error())
         was_valid = false;
 
     MirWaitHandle* wait_handle{nullptr};
@@ -350,29 +358,28 @@
 
 MirWaitHandle* MirSurface::configure_cursor(MirCursorConfiguration const* cursor)
 {
-    mp::CursorSetting setting;
+    auto setting = mcl::make_protobuf_object<mp::CursorSetting>();
 
     {
         std::unique_lock<decltype(mutex)> lock(mutex);
-        setting.mutable_surfaceid()->CopyFrom(surface.id());
+        setting->mutable_surfaceid()->CopyFrom(surface->id());
         if (cursor)
         {
             if (cursor->stream != nullptr)
             {
-                setting.mutable_buffer_stream()->set_value(cursor->stream->rpc_id().as_value());
-                setting.set_hotspot_x(cursor->hotspot_x);
-                setting.set_hotspot_y(cursor->hotspot_y);
+                setting->mutable_buffer_stream()->set_value(cursor->stream->rpc_id().as_value());
+                setting->set_hotspot_x(cursor->hotspot_x);
+                setting->set_hotspot_y(cursor->hotspot_y);
             }
             else if (cursor->name != mir_disabled_cursor_name)
             {
-                setting.set_name(cursor->name.c_str());
+                setting->set_name(cursor->name.c_str());
             }
-
         }
     }
     
     configure_cursor_wait_handle.expect_result();
-    server->configure_cursor(0, &setting, &void_response,
+    server->configure_cursor(0, setting.get(), void_response.get(),
         google::protobuf::NewCallback(this, &MirSurface::on_cursor_configured));
     
     return &configure_cursor_wait_handle;
@@ -392,14 +399,14 @@
 
     std::unique_lock<decltype(mutex)> lock(mutex);
 
-    mp::SurfaceSetting setting;
-    setting.mutable_surfaceid()->CopyFrom(surface.id());
-    setting.set_attrib(at);
-    setting.set_ivalue(value);
+    auto setting = mcl::make_protobuf_object<mp::SurfaceSetting>();
+    setting->mutable_surfaceid()->CopyFrom(surface->id());
+    setting->set_attrib(at);
+    setting->set_ivalue(value);
     lock.unlock();
 
     configure_wait_handle.expect_result();
-    server->configure_surface(0, &setting, &configure_result,
+    server->configure_surface(0, setting.get(), configure_result.get(),
               google::protobuf::NewCallback(this, &MirSurface::on_configured));
 
     return &configure_wait_handle;
@@ -421,12 +428,12 @@
         return false;
     }
 
-    mp::CoordinateTranslationRequest request;
+    auto request = mcl::make_protobuf_object<mp::CoordinateTranslationRequest>();
 
-    request.set_x(x);
-    request.set_y(y);
-    *request.mutable_surfaceid() = surface.id();
-    mp::CoordinateTranslationResponse response;
+    request->set_x(x);
+    request->set_y(y);
+    *request->mutable_surfaceid() = surface->id();
+    auto response = mcl::make_protobuf_object<mp::CoordinateTranslationResponse>();
 
     MirWaitHandle signal;
     signal.expect_result();
@@ -436,27 +443,27 @@
 
         debug->translate_surface_to_screen(
             nullptr,
-            &request,
-            &response,
+            request.get(),
+            response.get(),
             google::protobuf::NewCallback(&signal_response_received, &signal));
     }
 
     signal.wait_for_one();
 
-    *screen_x = response.x();
-    *screen_y = response.y();
-    return !response.has_error();
+    *screen_x = response->x();
+    *screen_y = response->y();
+    return !response->has_error();
 }
 
 void MirSurface::on_configured()
 {
     std::lock_guard<decltype(mutex)> lock(mutex);
 
-    if (configure_result.has_surfaceid() &&
-        configure_result.surfaceid().value() == surface.id().value() &&
-        configure_result.has_attrib())
+    if (configure_result->has_surfaceid() &&
+        configure_result->surfaceid().value() == surface->id().value() &&
+        configure_result->has_attrib())
     {
-        int a = configure_result.attrib();
+        int a = configure_result->attrib();
 
         switch (a)
         {
@@ -465,10 +472,10 @@
         case mir_surface_attrib_focus:
         case mir_surface_attrib_dpi:
         case mir_surface_attrib_preferred_orientation:
-            if (configure_result.has_ivalue())
-                attrib_cache[a] = configure_result.ivalue();
+            if (configure_result->has_ivalue())
+                attrib_cache[a] = configure_result->ivalue();
             else
-                assert(configure_result.has_error());
+                assert(configure_result->has_error());
             break;
         default:
             assert(false);
@@ -513,9 +520,9 @@
                                           std::placeholders::_1,
                                           context);
 
-        if (surface.fd_size() > 0 && handle_event_callback)
+        if (surface->fd_size() > 0 && handle_event_callback)
         {
-            auto input_dispatcher = input_platform->create_input_receiver(surface.fd(0),
+            auto input_dispatcher = input_platform->create_input_receiver(surface->fd(0),
                                                                           keymapper,
                                                                           handle_event_callback);
             input_thread = std::make_shared<md::ThreadedDispatcher>("Input dispatch", input_dispatcher);
@@ -587,7 +594,7 @@
 {
     {
         std::lock_guard<decltype(mutex)> lock(mutex);
-        if (modify_result.has_error())
+        if (modify_result->has_error())
         {
             // TODO return errors like lp:~vanvugt/mir/wait-result
         }
@@ -597,14 +604,14 @@
 
 MirWaitHandle* MirSurface::modify(MirSurfaceSpec const& spec)
 {
-    mp::SurfaceModifications mods;
+    auto mods = mcl::make_protobuf_object<mp::SurfaceModifications>();
 
     {
         std::unique_lock<decltype(mutex)> lock(mutex);
-        mods.mutable_surface_id()->set_value(surface.id().value());
+        mods->mutable_surface_id()->set_value(surface->id().value());
     }
 
-    auto const surface_specification = mods.mutable_surface_specification();
+    auto const surface_specification = mods->mutable_surface_specification();
 
     #define COPY_IF_SET(field)\
         if (spec.field.is_set())\
@@ -678,7 +685,7 @@
     }
 
     modify_wait_handle.expect_result();
-    server->modify_surface(0, &mods, &modify_result,
+    server->modify_surface(0, mods.get(), modify_result.get(),
               google::protobuf::NewCallback(this, &MirSurface::on_modified));
 
     return &modify_wait_handle;

=== modified file 'src/client/mir_surface.h'
--- src/client/mir_surface.h	2015-06-10 12:01:33 +0000
+++ src/client/mir_surface.h	2015-06-16 23:39:42 +0000
@@ -65,7 +65,7 @@
     MirSurfaceSpec(MirConnection* connection, int width, int height, MirPixelFormat format);
     MirSurfaceSpec(MirConnection* connection, MirSurfaceParameters const& params);
 
-    mir::protobuf::SurfaceParameters serialize() const;
+    std::unique_ptr<mir::protobuf::SurfaceParameters> serialize() const;
 
     struct AspectRatio { unsigned width; unsigned height; };
 
@@ -178,16 +178,15 @@
 
     mir::protobuf::DisplayServer::Stub* server{nullptr};
     mir::protobuf::Debug::Stub* debug{nullptr};
-    mir::protobuf::Surface surface;
-    mir::protobuf::BufferRequest buffer_request;
-    mir::protobuf::PersistentSurfaceId persistent_id;
+    std::unique_ptr<mir::protobuf::Surface> surface;
+    std::unique_ptr<mir::protobuf::PersistentSurfaceId> persistent_id;
     std::string error_message;
     std::string name;
-    mir::protobuf::Void void_response;
+    std::unique_ptr<mir::protobuf::Void> void_response;
 
     void on_modified();
     MirWaitHandle modify_wait_handle;
-    mir::protobuf::Void modify_result;
+    std::unique_ptr<mir::protobuf::Void> modify_result;
 
     MirConnection* const connection{nullptr};
 
@@ -201,7 +200,7 @@
     std::shared_ptr<mir::input::receiver::InputPlatform> const input_platform;
     std::shared_ptr<mir::input::receiver::XKBMapper> const keymapper;
 
-    mir::protobuf::SurfaceSetting configure_result;
+    std::unique_ptr<mir::protobuf::SurfaceSetting> configure_result;
 
     // Cache of latest SurfaceSettings returned from the server
     int attrib_cache[mir_surface_attribs];

=== modified file 'src/client/rpc/mir_protobuf_rpc_channel.cpp'
--- src/client/rpc/mir_protobuf_rpc_channel.cpp	2015-06-03 17:16:04 +0000
+++ src/client/rpc/mir_protobuf_rpc_channel.cpp	2015-06-16 23:39:42 +0000
@@ -25,6 +25,7 @@
 #include "../display_configuration.h"
 #include "../lifecycle_control.h"
 #include "../event_sink.h"
+#include "../make_protobuf_object.h"
 #include "mir/variable_length_array.h"
 #include "mir/events/event_private.h"
 
@@ -236,24 +237,24 @@
 
 void mclr::MirProtobufRpcChannel::process_event_sequence(std::string const& event)
 {
-    mir::protobuf::EventSequence seq;
-
-    seq.ParseFromString(event);
-
-    if (seq.has_display_configuration())
-    {
-        display_configuration->update_configuration(seq.display_configuration());
-    }
-
-    if (seq.has_lifecycle_event())
-    {
-        lifecycle_control->call_lifecycle_event_handler(seq.lifecycle_event().new_state());
-    }
-
-    int const nevents = seq.event_size();
+    auto seq = mcl::make_protobuf_object<mir::protobuf::EventSequence>();
+
+    seq->ParseFromString(event);
+
+    if (seq->has_display_configuration())
+    {
+        display_configuration->update_configuration(seq->display_configuration());
+    }
+
+    if (seq->has_lifecycle_event())
+    {
+        lifecycle_control->call_lifecycle_event_handler(seq->lifecycle_event().new_state());
+    }
+
+    int const nevents = seq->event_size();
     for (int i = 0; i != nevents; ++i)
     {
-        mir::protobuf::Event const& event = seq.event(i);
+        mir::protobuf::Event const& event = seq->event(i);
         if (event.has_raw())
         {
             std::string const& raw_event = event.raw();
@@ -320,7 +321,7 @@
      */
     std::lock_guard<decltype(read_mutex)> lock(read_mutex);
 
-    auto result = std::make_unique<mir::protobuf::wire::Result>();
+    auto result = mcl::make_protobuf_object<mir::protobuf::wire::Result>();
     try
     {
         uint16_t message_size;