Merge lp:~robertcarr/mir/socket-messenger-race into lp:~mir-team/mir/trunk

[Robert Carr]

socket_messenger: Protect whole_message with a lock.

It seems to me that socket messenger may be used from any thread (as a result of the shell generating various events, i.e. focus/unfocus).

This means that we need to protect whole_message with a lock.

[kevin gunn (kgunn72)]

should be ok from mirserver api, no break

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1078
http://jenkins.qa.ubuntu.com/job/mir-ci/1559/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-saucy-i386-build/2091
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-saucy-amd64-build/1976
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-saucy-amd64-ci/797/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/59
        deb: http://jenkins.qa.ubuntu.com/job/mir-saucy-armhf-ci/59/artifact/work/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Kevin DuBois (kdub)]

should retarget to development branch...

other than that though, the fix makes sense

review: Approve

[Alan Griffiths (alan-griffiths)]

I'm not sure this is adequate.

Unless I miss something, the lock needs to be held across the related send() and send_fds() calls originating in ProtobufMessageProcessor::send_response() - not just the send() call.

And that is best approached by changing interfaces so that the body and fds are passed to a single call.

review: Needs Fixing

=== modified file 'src/server/frontend/socket_messenger.cpp'
--- src/server/frontend/socket_messenger.cpp	2013-08-28 03:41:48 +0000
+++ src/server/frontend/socket_messenger.cpp	2013-09-23 15:06:12 +0000
@@ -49,6 +49,8 @@
         static_cast<unsigned char>((size >> 8) & 0xff),
         static_cast<unsigned char>((size >> 0) & 0xff)
     };
+    
+    std::unique_lock<std::mutex> lg(message_lock);
 
     whole_message.resize(sizeof header_bytes + size);
     std::copy(header_bytes, header_bytes + sizeof header_bytes, whole_message.begin());

=== modified file 'src/server/frontend/socket_messenger.h'
--- src/server/frontend/socket_messenger.h	2013-08-28 03:41:48 +0000
+++ src/server/frontend/socket_messenger.h	2013-09-23 15:06:12 +0000
@@ -42,6 +42,8 @@
 
 private:
     std::shared_ptr<boost::asio::local::stream_protocol::socket> socket;
+    
+    std::mutex message_lock;
     std::vector<char> whole_message;
 };
 }