mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 49404: Changed the SocketManager to pass outgoing "Sockets" by value.
Date Thu, 30 Jun 2016 01:06:17 GMT


> On June 30, 2016, 12:24 a.m., Benjamin Mahler wrote:
> > Looks good, thanks! Can you also sweep up the remaining Socket pointers that were
not as related to the race you found?

Actually, on second thought per our converstation. Could we fix this more minimally by adding
`new`s?

```
  socket->recv(data, size)
    .onAny(lambda::bind(
        &internal::ignore_recv_data,
        lambda::_1,
        new Socket(*socket), // XXX: new here
        data,
        size));
```

That would make backporting easier, the fix more obvious for posterity, and we can follow
up with a complete pointer sweep in a separate patch.


- Benjamin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49404/#review140079
-----------------------------------------------------------


On June 29, 2016, 11:56 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49404/
> -----------------------------------------------------------
> 
> (Updated June 29, 2016, 11:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Artem Harutyunyan.
> 
> 
> Bugs: MESOS-5748
>     https://issues.apache.org/jira/browse/MESOS-5748
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `Sockets` is already a reference-counted `shared_ptr` under the covers.
> By passing around `Sockets` by value, we avoid potentially deleting
> a reference while the same reference is in use by another function.
> 
> This fixes a rare race (segfault) between `link`/`send` and 
> `ignore_recv_data`.  If the peer of the socket exits between 
> establishing a connection and libprocess queuing a `MessageEncoder`, 
> `ignore_recv_data` may delete the `Socket` underneath the `link`/`send`.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/process.cpp 9bae71246e751e491be5a989eea8aca29c9aa751 
> 
> Diff: https://reviews.apache.org/r/49404/diff/
> 
> 
> Testing
> -------
> 
> make check (OSX)
> 
> 3rdparty/libprocess/libprocess-tests --gtest_filter="ProcessRemoteLinkTest.RemoteLink"
 --gtest_break_on_failure --gtest_repeat=10000
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message