Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap boost::asio::async_write to ensure lifetime of buffers #2240

Merged
merged 4 commits into from
Aug 29, 2019

Conversation

wezrule
Copy link
Contributor

@wezrule wezrule commented Aug 23, 2019

Introduce a new class nano::shared_const_buffer which @cryptocode found an example for here: https://www.boost.org/doc/libs/1_69_0/doc/html/boost_asio/example/cpp11/buffers/reference_counted.cpp This should alleviate any buffer memory not being captured or having sufficient lifetime during the boost::asio::async_write call. When building travis I check if boost::asio::async_write is called anywhere other than the new file which wraps it nano/lib/asio.hpp and error out with a message if it is.

I think any minor performance hits we have by always using a shared_ptr are outweighed by keeping the program correct, as this has bitten us in the past and I found yet more places (nano::rpc_request_processor::try_reconnect_and_execute_request) where this wasn't done properly even though we actively try make sure it is.

@wezrule wezrule added the quality improvements This item indicates the need for or supplies changes that improve maintainability label Aug 23, 2019
@wezrule wezrule added this to the V20.0 milestone Aug 23, 2019
@wezrule wezrule requested a review from cryptocode August 23, 2019 11:40
@wezrule wezrule self-assigned this Aug 23, 2019
@wezrule wezrule changed the title Wrap boost::asio::async_write to prevent out of scope errors Wrap boost::asio::async_write to ensure lifetime of buffers Aug 23, 2019
@cryptocode
Copy link
Contributor

For another PR, but would something similar work for reads? I.e. a shared_mutable_buffer class (using boost::asio::mutable_buffer)?

@wezrule wezrule requested a review from SergiySW August 23, 2019 12:43
@wezrule
Copy link
Contributor Author

wezrule commented Aug 23, 2019

The MutableBufferSequence has similar requirements to the ConstBufferSequence so I think it should be fine to add something similar for async_read
https://www.boost.org/doc/libs/1_67_0/doc/html/boost_asio/reference/MutableBufferSequence.html

@wezrule wezrule merged commit 86d20d5 into nanocurrency:master Aug 29, 2019
@wezrule wezrule deleted the prevent_raw_async_write branch August 29, 2019 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quality improvements This item indicates the need for or supplies changes that improve maintainability
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants