Skip to content

Commit 8f8bbc6

Browse files
jasnelladdaleax
authored andcommitted
src: use smart pointers for nghttp2 objects
Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #32551 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent f558a76 commit 8f8bbc6

File tree

2 files changed

+89
-69
lines changed

2 files changed

+89
-69
lines changed

src/node_http2.cc

+61-56
Original file line numberDiff line numberDiff line change
@@ -114,53 +114,56 @@ Http2Scope::~Http2Scope() {
114114
// uses a single TypedArray instance that is shared with the JavaScript side
115115
// to more efficiently pass values back and forth.
116116
Http2Options::Http2Options(Environment* env, nghttp2_session_type type) {
117-
nghttp2_option_new(&options_);
117+
nghttp2_option* option;
118+
CHECK_EQ(nghttp2_option_new(&option), 0);
119+
CHECK_NOT_NULL(option);
120+
options_.reset(option);
118121

119122
// Make sure closed connections aren't kept around, taking up memory.
120123
// Note that this breaks the priority tree, which we don't use.
121-
nghttp2_option_set_no_closed_streams(options_, 1);
124+
nghttp2_option_set_no_closed_streams(option, 1);
122125

123126
// We manually handle flow control within a session in order to
124127
// implement backpressure -- that is, we only send WINDOW_UPDATE
125128
// frames to the remote peer as data is actually consumed by user
126129
// code. This ensures that the flow of data over the connection
127130
// does not move too quickly and limits the amount of data we
128131
// are required to buffer.
129-
nghttp2_option_set_no_auto_window_update(options_, 1);
132+
nghttp2_option_set_no_auto_window_update(option, 1);
130133

131134
// Enable built in support for receiving ALTSVC and ORIGIN frames (but
132135
// only on client side sessions
133136
if (type == NGHTTP2_SESSION_CLIENT) {
134-
nghttp2_option_set_builtin_recv_extension_type(options_, NGHTTP2_ALTSVC);
135-
nghttp2_option_set_builtin_recv_extension_type(options_, NGHTTP2_ORIGIN);
137+
nghttp2_option_set_builtin_recv_extension_type(option, NGHTTP2_ALTSVC);
138+
nghttp2_option_set_builtin_recv_extension_type(option, NGHTTP2_ORIGIN);
136139
}
137140

138141
AliasedUint32Array& buffer = env->http2_state()->options_buffer;
139142
uint32_t flags = buffer[IDX_OPTIONS_FLAGS];
140143

141144
if (flags & (1 << IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE)) {
142145
nghttp2_option_set_max_deflate_dynamic_table_size(
143-
options_,
146+
option,
144147
buffer[IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE]);
145148
}
146149

147150
if (flags & (1 << IDX_OPTIONS_MAX_RESERVED_REMOTE_STREAMS)) {
148151
nghttp2_option_set_max_reserved_remote_streams(
149-
options_,
152+
option,
150153
buffer[IDX_OPTIONS_MAX_RESERVED_REMOTE_STREAMS]);
151154
}
152155

153156
if (flags & (1 << IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH)) {
154157
nghttp2_option_set_max_send_header_block_length(
155-
options_,
158+
option,
156159
buffer[IDX_OPTIONS_MAX_SEND_HEADER_BLOCK_LENGTH]);
157160
}
158161

159162
// Recommended default
160-
nghttp2_option_set_peer_max_concurrent_streams(options_, 100);
163+
nghttp2_option_set_peer_max_concurrent_streams(option, 100);
161164
if (flags & (1 << IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS)) {
162165
nghttp2_option_set_peer_max_concurrent_streams(
163-
options_,
166+
option,
164167
buffer[IDX_OPTIONS_PEER_MAX_CONCURRENT_STREAMS]);
165168
}
166169

@@ -179,27 +182,24 @@ Http2Options::Http2Options(Environment* env, nghttp2_session_type type) {
179182
// header pairs the session may accept. This is a hard limit.. that is,
180183
// if the remote peer sends more than this amount, the stream will be
181184
// automatically closed with an RST_STREAM.
182-
if (flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS)) {
185+
if (flags & (1 << IDX_OPTIONS_MAX_HEADER_LIST_PAIRS))
183186
SetMaxHeaderPairs(buffer[IDX_OPTIONS_MAX_HEADER_LIST_PAIRS]);
184-
}
185187

186188
// The HTTP2 specification places no limits on the number of HTTP2
187189
// PING frames that can be sent. In order to prevent PINGS from being
188190
// abused as an attack vector, however, we place a strict upper limit
189191
// on the number of unacknowledged PINGS that can be sent at any given
190192
// time.
191-
if (flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_PINGS)) {
193+
if (flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_PINGS))
192194
SetMaxOutstandingPings(buffer[IDX_OPTIONS_MAX_OUTSTANDING_PINGS]);
193-
}
194195

195196
// The HTTP2 specification places no limits on the number of HTTP2
196197
// SETTINGS frames that can be sent. In order to prevent PINGS from being
197198
// abused as an attack vector, however, we place a strict upper limit
198199
// on the number of unacknowledged SETTINGS that can be sent at any given
199200
// time.
200-
if (flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS)) {
201+
if (flags & (1 << IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS))
201202
SetMaxOutstandingSettings(buffer[IDX_OPTIONS_MAX_OUTSTANDING_SETTINGS]);
202-
}
203203

204204
// The HTTP2 specification places no limits on the amount of memory
205205
// that a session can consume. In order to prevent abuse, we place a
@@ -209,9 +209,8 @@ Http2Options::Http2Options(Environment* env, nghttp2_session_type type) {
209209
// created.
210210
// Important: The maxSessionMemory option in javascript is expressed in
211211
// terms of MB increments (i.e. the value 1 == 1 MB)
212-
if (flags & (1 << IDX_OPTIONS_MAX_SESSION_MEMORY)) {
212+
if (flags & (1 << IDX_OPTIONS_MAX_SESSION_MEMORY))
213213
SetMaxSessionMemory(buffer[IDX_OPTIONS_MAX_SESSION_MEMORY] * 1e6);
214-
}
215214
}
216215

217216
void Http2Session::Http2Settings::Init() {
@@ -421,42 +420,39 @@ Origins::Origins(Isolate* isolate,
421420
// Sets the various callback functions that nghttp2 will use to notify us
422421
// about significant events while processing http2 stuff.
423422
Http2Session::Callbacks::Callbacks(bool kHasGetPaddingCallback) {
424-
CHECK_EQ(nghttp2_session_callbacks_new(&callbacks), 0);
423+
nghttp2_session_callbacks* callbacks_;
424+
CHECK_EQ(nghttp2_session_callbacks_new(&callbacks_), 0);
425+
callbacks.reset(callbacks_);
425426

426427
nghttp2_session_callbacks_set_on_begin_headers_callback(
427-
callbacks, OnBeginHeadersCallback);
428+
callbacks_, OnBeginHeadersCallback);
428429
nghttp2_session_callbacks_set_on_header_callback2(
429-
callbacks, OnHeaderCallback);
430+
callbacks_, OnHeaderCallback);
430431
nghttp2_session_callbacks_set_on_frame_recv_callback(
431-
callbacks, OnFrameReceive);
432+
callbacks_, OnFrameReceive);
432433
nghttp2_session_callbacks_set_on_stream_close_callback(
433-
callbacks, OnStreamClose);
434+
callbacks_, OnStreamClose);
434435
nghttp2_session_callbacks_set_on_data_chunk_recv_callback(
435-
callbacks, OnDataChunkReceived);
436+
callbacks_, OnDataChunkReceived);
436437
nghttp2_session_callbacks_set_on_frame_not_send_callback(
437-
callbacks, OnFrameNotSent);
438+
callbacks_, OnFrameNotSent);
438439
nghttp2_session_callbacks_set_on_invalid_header_callback2(
439-
callbacks, OnInvalidHeader);
440+
callbacks_, OnInvalidHeader);
440441
nghttp2_session_callbacks_set_error_callback(
441-
callbacks, OnNghttpError);
442+
callbacks_, OnNghttpError);
442443
nghttp2_session_callbacks_set_send_data_callback(
443-
callbacks, OnSendData);
444+
callbacks_, OnSendData);
444445
nghttp2_session_callbacks_set_on_invalid_frame_recv_callback(
445-
callbacks, OnInvalidFrame);
446+
callbacks_, OnInvalidFrame);
446447
nghttp2_session_callbacks_set_on_frame_send_callback(
447-
callbacks, OnFrameSent);
448+
callbacks_, OnFrameSent);
448449

449450
if (kHasGetPaddingCallback) {
450451
nghttp2_session_callbacks_set_select_padding_callback(
451-
callbacks, OnSelectPadding);
452+
callbacks_, OnSelectPadding);
452453
}
453454
}
454455

455-
456-
Http2Session::Callbacks::~Callbacks() {
457-
nghttp2_session_callbacks_del(callbacks);
458-
}
459-
460456
void Http2Session::StopTrackingRcbuf(nghttp2_rcbuf* buf) {
461457
StopTrackingMemory(buf);
462458
}
@@ -500,9 +496,6 @@ Http2Session::Http2Session(Environment* env,
500496
bool hasGetPaddingCallback =
501497
padding_strategy_ != PADDING_STRATEGY_NONE;
502498

503-
nghttp2_session_callbacks* callbacks
504-
= callback_struct_saved[hasGetPaddingCallback ? 1 : 0].callbacks;
505-
506499
auto fn = type == NGHTTP2_SESSION_SERVER ?
507500
nghttp2_session_server_new3 :
508501
nghttp2_session_client_new3;
@@ -514,7 +507,14 @@ Http2Session::Http2Session(Environment* env,
514507
// of the options are out of acceptable range, which we should
515508
// be catching before it gets this far. Either way, crash if this
516509
// fails.
517-
CHECK_EQ(fn(&session_, callbacks, this, *opts, &alloc_info), 0);
510+
nghttp2_session* session;
511+
CHECK_EQ(fn(
512+
&session,
513+
callback_struct_saved[hasGetPaddingCallback ? 1 : 0].callbacks.get(),
514+
this,
515+
*opts,
516+
&alloc_info), 0);
517+
session_.reset(session);
518518

519519
outgoing_storage_.reserve(1024);
520520
outgoing_buffers_.reserve(32);
@@ -533,7 +533,9 @@ Http2Session::Http2Session(Environment* env,
533533
Http2Session::~Http2Session() {
534534
CHECK_EQ(flags_ & SESSION_STATE_HAS_SCOPE, 0);
535535
Debug(this, "freeing nghttp2 session");
536-
nghttp2_session_del(session_);
536+
// Explicitly reset session_ so the subsequent
537+
// current_nghttp2_memory_ check passes.
538+
session_.reset();
537539
CHECK_EQ(current_nghttp2_memory_, 0);
538540
js_fields_->~SessionJSFields();
539541
}
@@ -630,7 +632,7 @@ void Http2Session::Close(uint32_t code, bool socket_closed) {
630632
// make a best effort.
631633
if (!socket_closed) {
632634
Debug(this, "terminating session with code %d", code);
633-
CHECK_EQ(nghttp2_session_terminate_session(session_, code), 0);
635+
CHECK_EQ(nghttp2_session_terminate_session(session_.get(), code), 0);
634636
SendPendingData();
635637
} else if (stream_ != nullptr) {
636638
stream_->RemoveStreamListener(this);
@@ -670,7 +672,7 @@ inline Http2Stream* Http2Session::FindStream(int32_t id) {
670672
inline bool Http2Session::CanAddStream() {
671673
uint32_t maxConcurrentStreams =
672674
nghttp2_session_get_local_settings(
673-
session_, NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
675+
session_.get(), NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS);
674676
size_t maxSize =
675677
std::min(streams_.max_size(), static_cast<size_t>(maxConcurrentStreams));
676678
// We can add a new stream so long as we are less than the current
@@ -736,10 +738,10 @@ ssize_t Http2Session::ConsumeHTTP2Data() {
736738
// multiple side effects.
737739
Debug(this, "receiving %d bytes [wants data? %d]",
738740
read_len,
739-
nghttp2_session_want_read(session_));
741+
nghttp2_session_want_read(session_.get()));
740742
flags_ &= ~SESSION_STATE_NGHTTP2_RECV_PAUSED;
741743
ssize_t ret =
742-
nghttp2_session_mem_recv(session_,
744+
nghttp2_session_mem_recv(session_.get(),
743745
reinterpret_cast<uint8_t*>(stream_buf_.base) +
744746
stream_buf_offset_,
745747
read_len);
@@ -1438,7 +1440,7 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
14381440

14391441
if ((flags_ & SESSION_STATE_READING_STOPPED) &&
14401442
!(flags_ & SESSION_STATE_WRITE_IN_PROGRESS) &&
1441-
nghttp2_session_want_read(session_)) {
1443+
nghttp2_session_want_read(session_.get())) {
14421444
flags_ &= ~SESSION_STATE_READING_STOPPED;
14431445
stream_->ReadStart();
14441446
}
@@ -1466,16 +1468,16 @@ void Http2Session::OnStreamAfterWrite(WriteWrap* w, int status) {
14661468
// queue), but only if a write has not already been scheduled.
14671469
void Http2Session::MaybeScheduleWrite() {
14681470
CHECK_EQ(flags_ & SESSION_STATE_WRITE_SCHEDULED, 0);
1469-
if (UNLIKELY(session_ == nullptr))
1471+
if (UNLIKELY(!session_))
14701472
return;
14711473

1472-
if (nghttp2_session_want_write(session_)) {
1474+
if (nghttp2_session_want_write(session_.get())) {
14731475
HandleScope handle_scope(env()->isolate());
14741476
Debug(this, "scheduling write");
14751477
flags_ |= SESSION_STATE_WRITE_SCHEDULED;
14761478
BaseObjectPtr<Http2Session> strong_ref{this};
14771479
env()->SetImmediate([this, strong_ref](Environment* env) {
1478-
if (session_ == nullptr || !(flags_ & SESSION_STATE_WRITE_SCHEDULED)) {
1480+
if (!session_ || !(flags_ & SESSION_STATE_WRITE_SCHEDULED)) {
14791481
// This can happen e.g. when a stream was reset before this turn
14801482
// of the event loop, in which case SendPendingData() is called early,
14811483
// or the session was destroyed in the meantime.
@@ -1493,7 +1495,7 @@ void Http2Session::MaybeScheduleWrite() {
14931495

14941496
void Http2Session::MaybeStopReading() {
14951497
if (flags_ & SESSION_STATE_READING_STOPPED) return;
1496-
int want_read = nghttp2_session_want_read(session_);
1498+
int want_read = nghttp2_session_want_read(session_.get());
14971499
Debug(this, "wants read? %d", want_read);
14981500
if (want_read == 0 || (flags_ & SESSION_STATE_WRITE_IN_PROGRESS)) {
14991501
flags_ |= SESSION_STATE_READING_STOPPED;
@@ -1591,7 +1593,7 @@ uint8_t Http2Session::SendPendingData() {
15911593

15921594
// Part One: Gather data from nghttp2
15931595

1594-
while ((src_length = nghttp2_session_mem_send(session_, &src)) > 0) {
1596+
while ((src_length = nghttp2_session_mem_send(session_.get(), &src)) > 0) {
15951597
Debug(this, "nghttp2 has %d bytes to send", src_length);
15961598
CopyDataIntoOutgoing(src, src_length);
15971599
}
@@ -1716,7 +1718,7 @@ Http2Stream* Http2Session::SubmitRequest(
17161718
Http2Stream* stream = nullptr;
17171719
Http2Stream::Provider::Stream prov(options);
17181720
*ret = nghttp2_submit_request(
1719-
session_,
1721+
session_.get(),
17201722
prispec,
17211723
headers.data(),
17221724
headers.length(),
@@ -2486,9 +2488,9 @@ void Http2Session::Goaway(uint32_t code,
24862488
Http2Scope h2scope(this);
24872489
// the last proc stream id is the most recently created Http2Stream.
24882490
if (lastStreamID <= 0)
2489-
lastStreamID = nghttp2_session_get_last_proc_stream_id(session_);
2491+
lastStreamID = nghttp2_session_get_last_proc_stream_id(session_.get());
24902492
Debug(this, "submitting goaway");
2491-
nghttp2_submit_goaway(session_, NGHTTP2_FLAG_NONE,
2493+
nghttp2_submit_goaway(session_.get(), NGHTTP2_FLAG_NONE,
24922494
lastStreamID, code, data, len);
24932495
}
24942496

@@ -2677,13 +2679,16 @@ void Http2Session::AltSvc(int32_t id,
26772679
uint8_t* value,
26782680
size_t value_len) {
26792681
Http2Scope h2scope(this);
2680-
CHECK_EQ(nghttp2_submit_altsvc(session_, NGHTTP2_FLAG_NONE, id,
2682+
CHECK_EQ(nghttp2_submit_altsvc(session_.get(), NGHTTP2_FLAG_NONE, id,
26812683
origin, origin_len, value, value_len), 0);
26822684
}
26832685

26842686
void Http2Session::Origin(nghttp2_origin_entry* ov, size_t count) {
26852687
Http2Scope h2scope(this);
2686-
CHECK_EQ(nghttp2_submit_origin(session_, NGHTTP2_FLAG_NONE, ov, count), 0);
2688+
CHECK_EQ(nghttp2_submit_origin(
2689+
session_.get(),
2690+
NGHTTP2_FLAG_NONE,
2691+
ov, count), 0);
26872692
}
26882693

26892694
// Submits an AltSvc frame to be sent to the connected peer.

0 commit comments

Comments
 (0)