Implemented close correctly (#4002)

* Implemented close correctly

* Code review feedback - made several classes inal.

* clang-format
This commit is contained in:
Larry Osterman 2022-10-11 12:58:30 -07:00 committed by GitHub
parent b8330a1f5c
commit b5dd9ef05a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 43 additions and 25 deletions

View File

@ -62,7 +62,7 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets {
* Note: Some of these statistics are not available if the underlying transport supports native
* websockets.
*/
struct WebSocketStatistics
struct WebSocketStatistics final
{
/** @brief The number of WebSocket frames sent on this WebSocket. */
uint32_t FramesSent;
@ -172,11 +172,12 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets {
: FrameType{frameType}, IsFinalFrame{isFinalFrame}
{
}
virtual ~WebSocketFrame() {}
};
/** @brief Contains the contents of a WebSocket Text frame.*/
class WebSocketTextFrame : public WebSocketFrame,
public std::enable_shared_from_this<WebSocketTextFrame> {
class WebSocketTextFrame final : public WebSocketFrame,
public std::enable_shared_from_this<WebSocketTextFrame> {
friend Azure::Core::Http::WebSockets::_detail::WebSocketImplementation;
private:
@ -201,8 +202,8 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets {
};
/** @brief Contains the contents of a WebSocket Binary frame.*/
class WebSocketBinaryFrame : public WebSocketFrame,
public std::enable_shared_from_this<WebSocketBinaryFrame> {
class WebSocketBinaryFrame final : public WebSocketFrame,
public std::enable_shared_from_this<WebSocketBinaryFrame> {
friend Azure::Core::Http::WebSockets::_detail::WebSocketImplementation;
private:
@ -227,8 +228,9 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets {
};
/** @brief Contains the contents of a WebSocket Close frame.*/
class WebSocketPeerCloseFrame : public WebSocketFrame,
public std::enable_shared_from_this<WebSocketPeerCloseFrame> {
class WebSocketPeerCloseFrame final
: public WebSocketFrame,
public std::enable_shared_from_this<WebSocketPeerCloseFrame> {
friend Azure::Core::Http::WebSockets::_detail::WebSocketImplementation;
public:
@ -254,7 +256,7 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets {
}
};
struct WebSocketOptions : Azure::Core::_internal::ClientOptions
struct WebSocketOptions final : Azure::Core::_internal::ClientOptions
{
/**
* @brief The set of protocols which are supported by this client
@ -290,7 +292,7 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets {
WebSocketOptions() = default;
};
class WebSocket {
class WebSocket final {
public:
/** @brief Constructs a new instance of a WebSocket with the specified WebSocket options.
*
@ -311,13 +313,12 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets {
*/
void Open(Azure::Core::Context const& context = Azure::Core::Context{});
/** @brief Closes a WebSocket connection to the remote server gracefully.
*
* @param context Context for the operation.
/** @brief Closes a WebSocket connection to the remote server.
*/
void Close(Azure::Core::Context const& context = Azure::Core::Context{});
void Close();
/** @brief Closes a WebSocket connection to the remote server with additional context.
/** @brief Gracefully closes a WebSocket connection to the remote server with additional
* context.
*
* @param closeStatus 16 bit WebSocket error code.
* @param closeReason String describing the reason for closing the socket.

View File

@ -27,7 +27,13 @@
namespace Azure { namespace Core { namespace Http { namespace WebSockets {
void CurlWebSocketTransport::Close() { m_upgradedConnection->Shutdown(); }
void CurlWebSocketTransport::Close()
{
if (m_upgradedConnection)
{
m_upgradedConnection->Shutdown();
}
}
// Send an HTTP request to the remote server.
std::unique_ptr<RawResponse> CurlWebSocketTransport::Send(

View File

@ -21,11 +21,7 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets { names
{
m_socketImplementation->Open(context);
}
void WebSocket::Close(Azure::Core::Context const& context)
{
m_socketImplementation->Close(
static_cast<uint16_t>(WebSocketErrorCode::EndpointDisappearing), {}, context);
}
void WebSocket::Close() { m_socketImplementation->Close(); }
void WebSocket::Close(
uint16_t closeStatus,
std::string const& closeReason,

View File

@ -48,6 +48,8 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets { names
{
}
WebSocketImplementation::~WebSocketImplementation() { Close(); }
void WebSocketImplementation::Open(Azure::Core::Context const& context)
{
if (m_state != SocketState::Invalid && m_state != SocketState::Closed)
@ -197,6 +199,19 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets { names
m_headers.emplace(std::make_pair(header, headerValue));
}
void WebSocketImplementation::Close()
{
std::unique_lock<std::mutex> lock(m_stateMutex);
m_stateOwner = std::this_thread::get_id();
// Close the socket - after this point, the m_transport is invalid.
m_pingThread.Shutdown();
if (m_transport)
{
m_transport->Close();
}
m_state = SocketState::Closed;
}
void WebSocketImplementation::Close(
uint16_t closeStatus,
std::string const& closeReason,
@ -253,10 +268,8 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets { names
lock.lock();
m_stateOwner = std::this_thread::get_id();
}
// Close the socket - after this point, the m_transport is invalid.
m_pingThread.Shutdown();
m_transport->Close();
m_state = SocketState::Closed;
lock.unlock();
Close();
}
void WebSocketImplementation::SendFrame(

View File

@ -16,7 +16,7 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets { names
// Generator for random bytes. Used in WebSocketImplementation and tests.
std::vector<uint8_t> GenerateRandomBytes(size_t vectorSize);
class WebSocketImplementation {
class WebSocketImplementation final {
enum class SocketState
{
Invalid,
@ -30,8 +30,10 @@ namespace Azure { namespace Core { namespace Http { namespace WebSockets { names
WebSocketImplementation(
Azure::Core::Url const& remoteUrl,
_internal::WebSocketOptions const& options);
~WebSocketImplementation();
void Open(Azure::Core::Context const& context);
void Close();
void Close(
uint16_t closeStatus,
std::string const& closeReason,