windows: remember writes before connect and issue writes after connect #1439
Conversation
|
Thanks for working on this Andrius! I'll take a deeper look, but at a first glance I don't see the queue in the request being initialized, you sould also QUEUE_INIT(&req->queue). |
| @@ -821,6 +823,11 @@ int uv_tcp_write(uv_loop_t* loop, | |||
| int result; | |||
| DWORD bytes; | |||
|
|
|||
| if (!(handle->flags & UV_HANDLE_WRITABLE)) { | |||
| QUEUE_INSERT_TAIL(&handle->write_queue, &req->queue); | |||
There was a problem hiding this comment.
You'll also need to initialize req first.
|
Also, once the stream is connected successfully we need to do the actual writes, not just call the callbacks with UV_ECANCELED, is this something you plan to implement? |
|
Yes. I'm getting a Segmentation fault right now, so I want to get it out of the way before going on. |
|
It still immediately gets a segmentation fault on the QUEUE_REMOVE. |
|
You haven't initialized req->queue. |
|
Like with QUEUE_INIT? I haven't seen it initialized anywhere in the unix code.... but now I see. |
|
Looks reasonable, but still segfaults, I'll check the pointers now, maybe something changed. |
| @@ -1117,6 +1125,8 @@ void uv_process_tcp_connect_req(uv_loop_t* loop, uv_tcp_t* handle, | |||
| } | |||
| req->cb(req, uv_translate_sys_error(err)); | |||
|
|
|||
| uv__stream_flush_write_queue(&handle->write_queue, UV_ECANCELED); | |||
There was a problem hiding this comment.
You need to pass handle here, not handle->write_queue.
There was a problem hiding this comment.
Damn, I'm stupid.
Why doesn't it issue a warning :/
Fixes build
Makes the test reliably pass on Windows, otherwise the runtime aborts.
|
Andrius, if nothing unexpected happens I plan to make the 1.0.0 release by the end of next week, and since this modifies the ABI it would be nice to have it before that. Can you work on it by then? If not maybe I can find some time o help. Thanks! |
|
Changing the struct is an ABI change? |
|
Fantastic, thanks a lot! |
Yep. |
Remove duplicated code by directly calling uv_timer_stop
`fd_set`s are way too small for `select()` hack when stream's fd is bigger than 1023. Make `fd_set`s a part of `uv__stream_select_t` structure. fix #1461
It isn't really needed here.
- Remove the UV_HANDLE_ACTIVE flag. It's a duplicate from UV__HANDLE_ACTIVE, which was used solely on timers and loop watchers. - Avoid duplicated code when running timers by stopping the handle and rearming it with the repeat time, thus having a single place where the timers are added and removed to and from the RB tree, respectively.
The echo server shouldn't close the connection when there's an error on write. Instead simply echo the error message and allow the other side to close the connection. Also do a partial revert of 4d905fb where after_shutdown was removed. Fixes: 4d905fb "test: close stream immediately on error" Signed-off-by: Trevor Norris <[email protected]> Signed-off-by: Saúl Ibarra Corretgé <[email protected]>
Remove comments describing the API which are now part of the documentation, thus avoiding future comment rot (and removing the existing part).
|
Ping. |
Also close the IOCP handle for any loop. Closes #393
It seems that epoll_wait is implemented in glibc in terms of epoll_pwait and new architectures (like arm64) do not implement the epoll_wait syscall at all. So if epoll_wait errors with ENOSYS, just call epoll_pwait.
Improve timing precision by using QueryPerformanceCounter. This is part of the fix for Node.js' test-timers-first-fire.js.
|
@txdv any plans to resume this work? @trevnorris volunteered to take it over if you don't have the time. |
|
Here's a quick diff to get rid of all build warnings: diff --git a/src/win/internal.h b/src/win/internal.h
index d87402b..af6754c 100644
--- a/src/win/internal.h
+++ b/src/win/internal.h
@@ -120,6 +120,7 @@ extern UV_THREAD_LOCAL int uv__crt_assert_enabled;
* Streams: see stream-inl.h
*/
+void uv__stream_flush_write_queue(uv_stream_t* stream, int error);
/*
* TCP
diff --git a/src/win/tcp.c b/src/win/tcp.c
index dbebc06..38602eb 100644
--- a/src/win/tcp.c
+++ b/src/win/tcp.c
@@ -1126,7 +1126,7 @@ void uv_process_tcp_connect_req(uv_loop_t* loop, uv_tcp_t* handle,
}
req->cb(req, uv_translate_sys_error(err));
- uv__stream_flush_write_queue(handle, UV_ECANCELED);
+ uv__stream_flush_write_queue((uv_stream_t*)handle, UV_ECANCELED);
DECREASE_PENDING_REQ_COUNT(handle);
}
diff --git a/test/test-tcp-write-after-connect.c b/test/test-tcp-write-after-connect.c
index 02ff9c4..7d1d04a 100644
--- a/test/test-tcp-write-after-connect.c
+++ b/test/test-tcp-write-after-connect.c
@@ -42,7 +42,6 @@ static void connect_cb(uv_connect_t *req, int status) {
TEST_IMPL(tcp_write_after_connect) {
- int r;
struct sockaddr_in sa;
uv_buf_t buf;
buf = uv_buf_init("HELLO", 4); |
|
I'm working on it now. |
|
TTY works by default (it always immediately opens the stdin/stdout). Now only the pipe is in question. The pipe functionality is strange on windows... |
|
Andrius, can you open the PR against the v1.x branch? Now GH picks up all commits which are not yet in master. |
|
@txdv Can you link the new PR here? |
This makes it behave it like the linux code.
Currently this is not working, I get an segmentation error on windows when I try to remove something from the queue.
@saghul can you take a look on this, maybe i'm missing something obvious?
For some reason this gives me a segmentation error: https://github.com/txdv/libuv/compare/write_queue?expand=1#diff-dc490d8a5a8a41100ae824b52c0e496fR240
Also the returned pointer from QUEUE_DATA is absolute bullshit.