Skip to content

Commit d5fa4de

Browse files
committed
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 b171adc commit d5fa4de

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
@@ -120,20 +120,18 @@ TLSWrap::~TLSWrap() {
120120

121121

122122
void TLSWrap::MakePending() {
123-
write_item_queue_.MoveBack(&pending_write_items_);
123+
write_callback_scheduled_ = true;
124124
}
125125

126126

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

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

139137
return true;
@@ -303,7 +301,7 @@ void TLSWrap::EncOut() {
303301
return;
304302

305303
// Split-off queue
306-
if (established_ && !write_item_queue_.IsEmpty())
304+
if (established_ && current_write_ != nullptr)
307305
MakePending();
308306

309307
if (ssl_ == nullptr)
@@ -606,8 +604,9 @@ int TLSWrap::DoWrite(WriteWrap* w,
606604
}
607605
}
608606

609-
// Queue callback to execute it on next tick
610-
write_item_queue_.PushBack(new WriteItem(w));
607+
// Store the current write wrap
608+
CHECK_EQ(current_write_, nullptr);
609+
current_write_ = w;
611610
w->Dispatched();
612611

613612
// 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,
@@ -173,9 +160,8 @@ class TLSWrap : public AsyncWrap,
173160
BIO* enc_out_;
174161
crypto::NodeBIO* clear_in_;
175162
size_t write_size_;
176-
typedef ListHead<WriteItem, &WriteItem::member_> WriteItemList;
177-
WriteItemList write_item_queue_;
178-
WriteItemList pending_write_items_;
163+
WriteWrap* current_write_ = nullptr;
164+
bool write_callback_scheduled_ = false;
179165
bool started_;
180166
bool established_;
181167
bool shutdown_;

0 commit comments

Comments
 (0)