----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46024/#review128271 -----------------------------------------------------------
Mind adding joris and/or benh as reviewers? I'm happy to review but since they wrote the code I'd like them to be aware of this change. - Ben Mahler On April 11, 2016, 2:36 p.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46024/ > ----------------------------------------------------------- > > (Updated April 11, 2016, 2:36 p.m.) > > > Review request for mesos and Ben Mahler. > > > Repository: mesos > > > Description > ------- > > In `SocketManager::swap_implementing_socket`, the previous coding > accessed 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. > > > Diffs > ----- > > 3rdparty/libprocess/src/process.cpp > 5e9dcfdc52f3a8223bc43af149b8e1f5dbdf5b0a > > Diff: https://reviews.apache.org/r/46024/diff/ > > > Testing > ------- > > > Thanks, > > Neil Conway > >