Skip to content

Commit 96b0722

Browse files
addaleaxevanlucas
authored andcommitted
tls: refactor write queues away
The TLS implementation previously had two separate queues for WriteWrap instances, one for currently active and one for finishing writes (i.e. where the encrypted output is being written to the underlying socket). However, the streams implementation in Node doesn’t allow for more than one write to be active at a time; effectively, the only possible states were that: - No write was active. - The first write queue had one item, the second one was empty. - Only the second write queue had one item, the first one was empty. To reduce overhead and implementation complexity, remove these queues, and instead store a single `WriteWrap` pointer and keep track of whether its write callback should be called on the next invocation of `InvokeQueued()` or not (which is what being in the second queue previously effected). PR-URL: #17883 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]>
1 parent dcdb646 commit 96b0722

File tree

2 files changed

+12
-27
lines changed

2 files changed

+12
-27
lines changed

src/tls_wrap.cc

+10-11
Original file line numberDiff line numberDiff line change
@@ -118,20 +118,18 @@ TLSWrap::~TLSWrap() {
118118

119119

120120
void TLSWrap::MakePending() {
121-
write_item_queue_.MoveBack(&pending_write_items_);
121+
write_callback_scheduled_ = true;
122122
}
123123

124124

125125
bool TLSWrap::InvokeQueued(int status, const char* error_str) {
126-
if (pending_write_items_.IsEmpty())
126+
if (!write_callback_scheduled_)
127127
return false;
128128

129-
// Process old queue
130-
WriteItemList queue;
131-
pending_write_items_.MoveBack(&queue);
132-
while (WriteItem* wi = queue.PopFront()) {
133-
wi->w_->Done(status, error_str);
134-
delete wi;
129+
if (current_write_ != nullptr) {
130+
WriteWrap* w = current_write_;
131+
current_write_ = nullptr;
132+
w->Done(status, error_str);
135133
}
136134

137135
return true;
@@ -301,7 +299,7 @@ void TLSWrap::EncOut() {
301299
return;
302300

303301
// Split-off queue
304-
if (established_ && !write_item_queue_.IsEmpty())
302+
if (established_ && current_write_ != nullptr)
305303
MakePending();
306304

307305
if (ssl_ == nullptr)
@@ -619,8 +617,9 @@ int TLSWrap::DoWrite(WriteWrap* w,
619617
}
620618
}
621619

622-
// Queue callback to execute it on next tick
623-
write_item_queue_.PushBack(new WriteItem(w));
620+
// Store the current write wrap
621+
CHECK_EQ(current_write_, nullptr);
622+
current_write_ = w;
624623
w->Dispatched();
625624

626625
// Write queued data

src/tls_wrap.h

+2-16
Original file line numberDiff line numberDiff line change
@@ -90,19 +90,6 @@ class TLSWrap : public AsyncWrap,
9090
// Maximum number of buffers passed to uv_write()
9191
static const int kSimultaneousBufferCount = 10;
9292

93-
// Write callback queue's item
94-
class WriteItem {
95-
public:
96-
explicit WriteItem(WriteWrap* w) : w_(w) {
97-
}
98-
~WriteItem() {
99-
w_ = nullptr;
100-
}
101-
102-
WriteWrap* w_;
103-
ListNode<WriteItem> member_;
104-
};
105-
10693
TLSWrap(Environment* env,
10794
Kind kind,
10895
StreamBase* stream,
@@ -174,9 +161,8 @@ class TLSWrap : public AsyncWrap,
174161
BIO* enc_out_;
175162
crypto::NodeBIO* clear_in_;
176163
size_t write_size_;
177-
typedef ListHead<WriteItem, &WriteItem::member_> WriteItemList;
178-
WriteItemList write_item_queue_;
179-
WriteItemList pending_write_items_;
164+
WriteWrap* current_write_ = nullptr;
165+
bool write_callback_scheduled_ = false;
180166
bool started_;
181167
bool established_;
182168
bool shutdown_;

0 commit comments

Comments
 (0)