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.

Sign up for GitHub

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

fix: Handle exception closing stream for unavailable adapter on windows. #427

Merged
merged 5 commits into from
Jul 25, 2024
Merged

fix: Handle exception closing stream for unavailable adapter on windows. #427

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2e3b6a0
fix: Handle exception closing stream for unavailable adapter on windows.
kinyoklion Jul 24, 2024
a2fda98
Correct
kinyoklion Jul 24, 2024
70b49cd
Merge branch 'main' into rlamb/sc-251111/stream-close-crash
kinyoklion Jul 24, 2024
743ed9e
Add comment explaining the condition.
kinyoklion Jul 24, 2024
5e4e5c5
Merge branch 'rlamb/sc-251111/stream-close-crash' of github.com:launc…
kinyoklion Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions libs/server-sent-events/src/client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#include <boost/asio/ip/tcp.hpp>

Check notice on line 1 in libs/server-sent-events/src/client.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

Run clang-format on libs/server-sent-events/src/client.cpp 

File libs/server-sent-events/src/client.cpp does not conform to Custom style guidelines. (lines 356)
#include <boost/asio/steady_timer.hpp>
#include <boost/asio/strand.hpp>
#include <boost/asio/use_future.hpp>
Expand Down Expand Up @@ -60,11 +60,11 @@
using response = http::response<body>;

public:
FoxyClient(boost::asio::any_io_executor executor,

Check warning on line 63 in libs/server-sent-events/src/client.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sent-events/src/client.cpp:63:45 [performance-unnecessary-value-param] 

the parameter 'executor' is copied for each invocation but only used as a const reference; consider making it a const reference
http::request<http::string_body> req,
std::string host,
std::string port,
std::optional<std::chrono::milliseconds> connect_timeout,

Check warning on line 67 in libs/server-sent-events/src/client.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sent-events/src/client.cpp:67:16 [bugprone-easily-swappable-parameters] 

4 adjacent parameters of 'FoxyClient' of similar type ('std::optional<std::chrono::milliseconds>') are easily swapped by mistake
std::optional<std::chrono::milliseconds> read_timeout,
std::optional<std::chrono::milliseconds> write_timeout,
std::optional<std::chrono::milliseconds> initial_reconnect_delay,
Expand All @@ -88,7 +88,7 @@
backoff_(
initial_reconnect_delay.value_or(kDefaultInitialReconnectDelay),
kDefaultMaxBackoffDelay),
backoff_timer_(std::move(executor)),

Check warning on line 91 in libs/server-sent-events/src/client.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sent-events/src/client.cpp:91:26 [performance-move-const-arg] 

passing result of std::move() as a const reference argument; no move will actually happen
last_read_(std::nullopt),
shutting_down_(false) {
create_session();
Expand Down Expand Up @@ -123,7 +123,7 @@
void async_backoff(std::string const& reason) {
backoff_.fail();

if (auto id = body_parser_->get().body().last_event_id()) {

Check warning on line 126 in libs/server-sent-events/src/client.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sent-events/src/client.cpp:126:23 [bugprone-unchecked-optional-access] 

unchecked access to optional value
if (!id->empty()) {
last_event_id_ = id;
}
Expand Down Expand Up @@ -154,12 +154,12 @@

void async_connect() override {
boost::asio::post(
session_->get_executor(),

Check warning on line 157 in libs/server-sent-events/src/client.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sent-events/src/client.cpp:157:13 [bugprone-unchecked-optional-access] 

unchecked access to optional value
beast::bind_front_handler(&FoxyClient::do_run, shared_from_this()));
}

void do_run() {
session_->async_connect(

Check warning on line 162 in libs/server-sent-events/src/client.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sent-events/src/client.cpp:162:9 [bugprone-unchecked-optional-access] 

unchecked access to optional value
host_, port_,
beast::bind_front_handler(&FoxyClient::on_connect,
shared_from_this()));
Expand All @@ -178,8 +178,8 @@
} else {
req_.erase("last-event-id");
}
session_->opts.timeout = write_timeout_.value_or(kNoTimeout);

Check warning on line 181 in libs/server-sent-events/src/client.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sent-events/src/client.cpp:181:9 [bugprone-unchecked-optional-access] 

unchecked access to optional value
session_->async_write(req_,

Check warning on line 182 in libs/server-sent-events/src/client.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sent-events/src/client.cpp:182:9 [bugprone-unchecked-optional-access] 

unchecked access to optional value
beast::bind_front_handler(&FoxyClient::on_write,
shared_from_this()));
}
Expand All @@ -193,7 +193,7 @@
return async_backoff(ec.what());
}

session_->opts.timeout = read_timeout_.value_or(kNoTimeout);

Check warning on line 196 in libs/server-sent-events/src/client.cpp

View workflow job for this annotation

GitHub Actions / cpp-linter

/libs/server-sent-events/src/client.cpp:196:9 [bugprone-unchecked-optional-access] 

unchecked access to optional value
session_->async_read_header(
*body_parser_, beast::bind_front_handler(&FoxyClient::on_headers,
shared_from_this()));
Expand Down Expand Up @@ -341,13 +341,21 @@
// If any backoff is taking place, cancel that as well.
backoff_timer_.cancel();
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function can theoretically throw as well, but I cannot find any confirmation of a condition where it actually throws.


// Cancels the outstanding read.
if (session_->stream.is_ssl()) {
session_->stream.ssl().next_layer().cancel();
} else {
session_->stream.plain().cancel();
try {
// Cancels the outstanding read.
if (session_->stream.is_ssl()) {
session_->stream.ssl().next_layer().cancel();
} else {
session_->stream.plain().cancel();
}
} catch (boost::system::system_error const& err) {
// The stream is likely already closed. Potentially the network
// interface that was associated with the stream is no longer
// available.
logger_("exception closing stream: " + std::string(err.what()));
}


// Ideally we would call session_->async_shutdown() here to gracefully
// terminate the SSL session. For unknown reasons, this call appears to
// hang indefinitely and never complete until the SDK client is
Expand Down
Loading