Skip to content

Commit

Permalink
tls: refactor write queues away
Browse files Browse the repository at this point in the history
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]>
  • Loading branch information
addaleax authored and evanlucas committed Jan 24, 2018
1 parent dcdb646 commit 96b0722
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 27 deletions.
21 changes: 10 additions & 11 deletions src/tls_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,20 +118,18 @@ TLSWrap::~TLSWrap() {


void TLSWrap::MakePending() {
write_item_queue_.MoveBack(&pending_write_items_);
write_callback_scheduled_ = true;
}


bool TLSWrap::InvokeQueued(int status, const char* error_str) {
if (pending_write_items_.IsEmpty())
if (!write_callback_scheduled_)
return false;

// Process old queue
WriteItemList queue;
pending_write_items_.MoveBack(&queue);
while (WriteItem* wi = queue.PopFront()) {
wi->w_->Done(status, error_str);
delete wi;
if (current_write_ != nullptr) {
WriteWrap* w = current_write_;
current_write_ = nullptr;
w->Done(status, error_str);
}

return true;
Expand Down Expand Up @@ -301,7 +299,7 @@ void TLSWrap::EncOut() {
return;

// Split-off queue
if (established_ && !write_item_queue_.IsEmpty())
if (established_ && current_write_ != nullptr)
MakePending();

if (ssl_ == nullptr)
Expand Down Expand Up @@ -619,8 +617,9 @@ int TLSWrap::DoWrite(WriteWrap* w,
}
}

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

// Write queued data
Expand Down
18 changes: 2 additions & 16 deletions src/tls_wrap.h
Original file line number Diff line number Diff line change
Expand Up @@ -90,19 +90,6 @@ class TLSWrap : public AsyncWrap,
// Maximum number of buffers passed to uv_write()
static const int kSimultaneousBufferCount = 10;

// Write callback queue's item
class WriteItem {
public:
explicit WriteItem(WriteWrap* w) : w_(w) {
}
~WriteItem() {
w_ = nullptr;
}

WriteWrap* w_;
ListNode<WriteItem> member_;
};

TLSWrap(Environment* env,
Kind kind,
StreamBase* stream,
Expand Down Expand Up @@ -174,9 +161,8 @@ class TLSWrap : public AsyncWrap,
BIO* enc_out_;
crypto::NodeBIO* clear_in_;
size_t write_size_;
typedef ListHead<WriteItem, &WriteItem::member_> WriteItemList;
WriteItemList write_item_queue_;
WriteItemList pending_write_items_;
WriteWrap* current_write_ = nullptr;
bool write_callback_scheduled_ = false;
bool started_;
bool established_;
bool shutdown_;
Expand Down

0 comments on commit 96b0722

Please sign in to comment.