Convert to boost::beast #2348
Convert to boost::beast #2348
Conversation
Jenkins Build SummaryBuilt from this commit Built at 20180125 - 18:44:01 Test Results
|
|
Windows unit test asserts |
|
Which assert? |
|
Oh, inside boost/beast. Interesting I'm not seeing that on macOS. |
|
This is the line that is asserting: https://github.com/boostorg/beast/blob/develop/include/boost/beast/websocket/impl/read.ipp#L638 Notes:
Questions:
|
|
@HowardHinnant I do exercise that line in linux. I'm currently trying to set up a windows VM that I can compile rippled on. |
|
Interesting, thanks. I'll try to figure out why it isn't exercised on macOS. |
|
I am mistaken above: This assert is exercised on macOS, but does not fire. |
|
This PR looks fine to me. But I'm holding off on the approve until we learn more about the Windows unittest failure. I've taken a cursory look, but have been unsuccessful at a diagnosis using only macOS. |
|
Pushed a fix for the window's assert (thanks you @miguelportilla!) |
src/test/jtx/impl/WSClient.cpp
Outdated
| @@ -117,7 +117,9 @@ class WSClientImpl : public WSClient | |||
| error_code ec; | |||
| if (!peerClosed_) | |||
| { | |||
| ws_.async_close({}, [&](error_code ec) { stream_.close(ec); }); | |||
| ws_.async_close({}, strand_.wrap([&](error_code ec) { | |||
| stream_.close(ec); | |||
There was a problem hiding this comment.
It is not necessary to call stream_.close(ec) as its already closed with ws_async_close. However, calling stream_.cancel(ec) is a good idea to stop all asynchronous operations associated with the socket.
seelabs
added
the
Ready to merge
| { | ||
| ec.assign(0, ec.category()); | ||
| return {{const_buffers_type{ | ||
| body_string_.data(), body_string_.size()}, false}}; | ||
| } |
There was a problem hiding this comment.
Looks good. I wonder if storing the buffer might be a better option.
eg.
class writer
{
boost::asio::const_buffer buffer_;
public:
using const_buffers_type =
boost::asio::const_buffer;
template<bool isRequest, class Fields>
explicit
writer(boost::beast::http::message<isRequest,
json_body, Fields> const& msg)
{
auto s {to_string(msg.body())};
buffer_ = const_buffers_type{s.data(), s.size()};
}
void
init(boost::beast::error_code& ec)
{
ec.assign(0, ec.category());
}
boost::optional<std::pair<const_buffers_type, bool>>
get(boost::beast::error_code& ec)
{
ec.assign(0, ec.category());
return {{buffer_, false}};
}
};
There was a problem hiding this comment.
const_buffer doesn't own it's memory, so I don't think that works (it would point to the re-claimed string memory).
There was a problem hiding this comment.
You're correct, I forgot const_buffer doesn't own the memory and cheap to construct.
|
In 0.90.0-b5 |
After this patch is accepted rippled will require boost 1.66 or later.