Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 281fd7a

Browse filesBrowse files
ywave620danielleadams
authored andcommitted
src,stream: improve DoWrite() and Write()
Clarify StreamResource::DoWrite() interface definition. Fix error-prone Http2Stream::DoWrite(). Change StreamBase::Write() to allow caller to avoid inefficient write behavior. PR-URL: #44434 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
1 parent 99a2c16 commit 281fd7a
Copy full SHA for 281fd7a

File tree

Expand file treeCollapse file tree

4 files changed

+26
-18
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+26
-18
lines changed
Open diff view settings
Collapse file

‎src/node_http2.cc‎

Copy file name to clipboardExpand all lines: src/node_http2.cc
+1-2Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2375,8 +2375,7 @@ int Http2Stream::DoWrite(WriteWrap* req_wrap,
23752375
CHECK_NULL(send_handle);
23762376
Http2Scope h2scope(this);
23772377
if (!is_writable() || is_destroyed()) {
2378-
req_wrap->Done(UV_EOF);
2379-
return 0;
2378+
return UV_EOF;
23802379
}
23812380
Debug(this, "queuing %d buffers to send", nbufs);
23822381
for (size_t i = 0; i < nbufs; ++i) {
Collapse file

‎src/stream_base-inl.h‎

Copy file name to clipboardExpand all lines: src/stream_base-inl.h
+6-6Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -160,11 +160,11 @@ int StreamBase::Shutdown(v8::Local<v8::Object> req_wrap_obj) {
160160
return err;
161161
}
162162

163-
StreamWriteResult StreamBase::Write(
164-
uv_buf_t* bufs,
165-
size_t count,
166-
uv_stream_t* send_handle,
167-
v8::Local<v8::Object> req_wrap_obj) {
163+
StreamWriteResult StreamBase::Write(uv_buf_t* bufs,
164+
size_t count,
165+
uv_stream_t* send_handle,
166+
v8::Local<v8::Object> req_wrap_obj,
167+
bool skip_try_write) {
168168
Environment* env = stream_env();
169169
int err;
170170

@@ -173,7 +173,7 @@ StreamWriteResult StreamBase::Write(
173173
total_bytes += bufs[i].len;
174174
bytes_written_ += total_bytes;
175175

176-
if (send_handle == nullptr) {
176+
if (send_handle == nullptr && !skip_try_write) {
177177
err = DoTryWrite(&bufs, &count);
178178
if (err != 0 || count == 0) {
179179
return StreamWriteResult { false, err, nullptr, total_bytes, {} };
Collapse file

‎src/stream_base.cc‎

Copy file name to clipboardExpand all lines: src/stream_base.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ int StreamBase::WriteString(const FunctionCallbackInfo<Value>& args) {
335335
}
336336
}
337337

338-
StreamWriteResult res = Write(&buf, 1, send_handle, req_wrap_obj);
338+
StreamWriteResult res = Write(&buf, 1, send_handle, req_wrap_obj, try_write);
339339
res.bytes += synchronously_written;
340340

341341
SetWriteResult(res);
Collapse file

‎src/stream_base.h‎

Copy file name to clipboardExpand all lines: src/stream_base.h
+18-9Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -244,14 +244,19 @@ class StreamResource {
244244
// `*bufs` and `*count` accordingly. This is a no-op by default.
245245
// Return 0 for success and a libuv error code for failures.
246246
virtual int DoTryWrite(uv_buf_t** bufs, size_t* count);
247-
// Initiate a write of data. If the write completes synchronously, return 0 on
248-
// success (with bufs modified to indicate how much data was consumed) or a
249-
// libuv error code on failure. If the write will complete asynchronously,
250-
// return 0. When the write completes asynchronously, call req_wrap->Done()
251-
// with 0 on success (with bufs modified to indicate how much data was
252-
// consumed) or a libuv error code on failure. Do not call req_wrap->Done() if
253-
// the write completes synchronously, that is, it should never be called
254-
// before DoWrite() has returned.
247+
// Initiate a write of data.
248+
// Upon an immediate failure, a libuv error code is returned,
249+
// w->Done() will never be called and caller should free `bufs`.
250+
// Otherwise, 0 is returned and w->Done(status) will be called
251+
// with status set to either
252+
// (1) 0 after all data are written, or
253+
// (2) a libuv error code when an error occurs
254+
// in either case, w->Done() will never be called before DoWrite() returns.
255+
// When 0 is returned:
256+
// (1) memory specified by `bufs` and `count` must remain valid until
257+
// w->Done() gets called.
258+
// (2) `bufs` might or might not be changed, caller should not rely on this.
259+
// (3) `bufs` should be freed after w->Done() gets called.
255260
virtual int DoWrite(WriteWrap* w,
256261
uv_buf_t* bufs,
257262
size_t count,
@@ -343,13 +348,17 @@ class StreamBase : public StreamResource {
343348
// WriteWrap object (that was created in JS), or a new one will be created.
344349
// This will first try to write synchronously using `DoTryWrite()`, then
345350
// asynchronously using `DoWrite()`.
351+
// Caller can pass `skip_try_write` as true if it has already called
352+
// `DoTryWrite()` and ends up with a partial write, or it knows that the
353+
// write is too large to finish synchronously.
346354
// If the return value indicates a synchronous completion, no callback will
347355
// be invoked.
348356
inline StreamWriteResult Write(
349357
uv_buf_t* bufs,
350358
size_t count,
351359
uv_stream_t* send_handle = nullptr,
352-
v8::Local<v8::Object> req_wrap_obj = v8::Local<v8::Object>());
360+
v8::Local<v8::Object> req_wrap_obj = v8::Local<v8::Object>(),
361+
bool skip_try_write = false);
353362

354363
// These can be overridden by subclasses to get more specific wrap instances.
355364
// For example, a subclass Foo could create a FooWriteWrap or FooShutdownWrap

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.