libuv / libuv

Cross-platform asynchronous I/O
https://libuv.org/
MIT License
24.41k stars 3.61k forks source link

uv_write documentation unclear about buffer lifetime and layout requirements. #1072

Closed nikhilm closed 8 years ago

nikhilm commented 8 years ago

1) uv_write2, and by extension, uv_write, perform a copy of bufs into an internal buffer of the uv_write_t. This is done via a memcpy starting at bufs and being n * sizeof(bufs[0]), which requires that the buffers be contiguous and equally sized. The documentation at http://docs.libuv.org/en/v1.x/stream.html does not capture this requirement. This could lead to corruption bugs in the bytes being written if the buffers were not contiguous.

2) The implementations either copy buffers into a temporary buffer, or write them out synchronously, so the buffer is free to be released after the call, and does not necessitate waiting until the callback indicating completion. The doc should clarify that it is OK to release the buffer immediately.

I guess (2) would lock libuv into a possible non-optimal implementation, but the doc should at least then clarify that the buffer(s) may be released after the callback is invoked.

vcunat commented 8 years ago

No, bufs is an array of uv_buf_t which are tiny structs, each containing a pointer and size. I see no implied requirement on the pointed-to buffers.

I personally believe it's unavoidable to have to postpone releasing the buffers until after completion, unless libuv wanted to pay a nontrivial performance penalty, which might also noticeably hit my use case of knot resolver, among others.

Docs should clarify this anyway.

saghul commented 8 years ago

@vcunat is correct. What we copy is the uv_buf_t structures not the actual content of the buffers. Those can be of different sizes or whatever, there is no requirement.

I agree we should clarify this.

saghul commented 8 years ago

To clarify 2), libuv does try to do the write immediately, which may succeed, but the completion status is always deferred to the callback, in order to provide a consistent API. The buffers can only be freed after the callback is called, and they should be held by the user, since libuv nullifies its reference.

vcunat commented 8 years ago

See #1074.