Repository: mesos Updated Branches: refs/heads/master ede896541 -> 0426ffbbf
Fixed a locking issue in the SocketManager. In `SocketManager::swap_implementing_socket`, the `sockets` instance variable in two CHECKs without acquiring `mutex`. This is unsafe in general: `mutex` should be acquired before accessing instance variables like `sockets` (unless you know all such concurrent accesses will be read-only etc.). As it happens, the previous coding was not unsafe because all the call-sites of `SocketManager::swap_implementing_socket` acquire `mutex` before invoking it. But because `swap_implementing_socket` also acquires `mutex`, clearly its API contract allows it to be invoked without holding `mutex`, in which case the CHECKs would likely be unsafe. Review: https://reviews.apache.org/r/46024/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/0426ffbb Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/0426ffbb Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/0426ffbb Branch: refs/heads/master Commit: 0426ffbbf811245003651a5f0a8c11894a46e2c3 Parents: ede8965 Author: Neil Conway <neil.con...@gmail.com> Authored: Mon Apr 11 19:31:58 2016 -0700 Committer: Benjamin Mahler <bmah...@apache.org> Committed: Mon Apr 11 19:31:58 2016 -0700 ---------------------------------------------------------------------- 3rdparty/libprocess/src/process.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/0426ffbb/3rdparty/libprocess/src/process.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/process.cpp b/3rdparty/libprocess/src/process.cpp index 5e9dcfd..3f2f836 100644 --- a/3rdparty/libprocess/src/process.cpp +++ b/3rdparty/libprocess/src/process.cpp @@ -2092,11 +2092,11 @@ void SocketManager::swap_implementing_socket(const Socket& from, Socket* to) const int from_fd = from.get(); const int to_fd = to->get(); - // Make sure the 'from' and 'to' are valid to swap. - CHECK(sockets.count(from_fd) > 0); - CHECK(sockets.count(to_fd) == 0); - synchronized (mutex) { + // Make sure 'from' and 'to' are valid to swap. + CHECK(sockets.count(from_fd) > 0); + CHECK(sockets.count(to_fd) == 0); + sockets.erase(from_fd); sockets[to_fd] = to;
