Skip to content

Commit 5a5067b

Browse files
authored
add refcount for HttpMessage and remove ownMessage (aws#166)
*Issue #, if available:* aws/aws-iot-device-sdk-cpp-v2#146 If the underlying http_message get destroyed before the destructor of HttpMessage, it may crash when the destructor accesses the underlying http_message. *Description of changes:* - Keep the underlying http_message alive until the destructor called
1 parent 3a450f6 commit 5a5067b

File tree

2 files changed

+16
-10
lines changed

2 files changed

+16
-10
lines changed

include/aws/crt/http/HttpRequestResponse.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,12 +59,11 @@ namespace Aws
5959
struct aws_http_message *GetUnderlyingMessage() const noexcept { return m_message; }
6060

6161
protected:
62-
HttpMessage(Allocator *allocator, struct aws_http_message *message, bool ownsMessage = true) noexcept;
62+
HttpMessage(Allocator *allocator, struct aws_http_message *message) noexcept;
6363

6464
Allocator *m_allocator;
6565
struct aws_http_message *m_message;
6666
std::shared_ptr<Aws::Crt::Io::InputStream> m_bodyStream;
67-
bool m_ownsMessage;
6867
};
6968

7069
/**

source/http/HttpRequestResponse.cpp

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,14 @@ namespace Aws
1616
namespace Http
1717
{
1818

19-
HttpMessage::HttpMessage(Allocator *allocator, struct aws_http_message *message, bool ownsMessage) noexcept
20-
: m_allocator(allocator), m_message(message), m_bodyStream(nullptr), m_ownsMessage(ownsMessage)
19+
HttpMessage::HttpMessage(Allocator *allocator, struct aws_http_message *message) noexcept
20+
: m_allocator(allocator), m_message(message), m_bodyStream(nullptr)
2121
{
22+
if (message)
23+
{
24+
// Acquire a refcount to keep the message alive until this object dies.
25+
aws_http_message_acquire(this->m_message);
26+
}
2227
}
2328

2429
HttpMessage::~HttpMessage()
@@ -31,10 +36,8 @@ namespace Aws
3136
aws_input_stream_destroy(old_stream);
3237
}
3338

34-
if (m_ownsMessage)
35-
{
36-
aws_http_message_destroy(m_message);
37-
}
39+
aws_http_message_release(m_message);
40+
3841
m_message = nullptr;
3942
}
4043
}
@@ -92,12 +95,14 @@ namespace Aws
9295
}
9396

9497
HttpRequest::HttpRequest(Allocator *allocator)
95-
: HttpMessage(allocator, aws_http_message_new_request(allocator), true)
98+
: HttpMessage(allocator, aws_http_message_new_request(allocator))
9699
{
100+
// Releas the refcount as it created, since HttpMessage is taking the ownership
101+
aws_http_message_release(this->m_message);
97102
}
98103

99104
HttpRequest::HttpRequest(Allocator *allocator, struct aws_http_message *message)
100-
: HttpMessage(allocator, message, false)
105+
: HttpMessage(allocator, message)
101106
{
102107
}
103108

@@ -136,6 +141,8 @@ namespace Aws
136141
HttpResponse::HttpResponse(Allocator *allocator)
137142
: HttpMessage(allocator, aws_http_message_new_response(allocator))
138143
{
144+
// Releas the refcount as it created, since HttpMessage is taking the ownership
145+
aws_http_message_release(this->m_message);
139146
}
140147

141148
Optional<int> HttpResponse::GetResponseCode() const noexcept

0 commit comments

Comments
 (0)