Skip to content

Commit 8601d0c

Browse files
manojampalammanojampalam
committed
Various fixes around _on_select for each io to record error (instead of returning it on select)
1 parent 809d398 commit 8601d0c

File tree

5 files changed

+130
-106
lines changed

5 files changed

+130
-106
lines changed

‎contrib/win32/win32compat/fileio.c

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -594,19 +594,28 @@ fileio_fdopen(struct w32_io* pio, const char *mode) {
594594
return _fdopen(fd, mode);
595595
}
596596

597-
int
597+
void
598598
fileio_on_select(struct w32_io* pio, BOOL rd) {
599599

600600
if (!rd)
601-
return 0;
601+
return;
602602

603603
if (!pio->read_details.pending && !fileio_is_io_available(pio, rd))
604-
if (FILETYPE(pio) == FILE_TYPE_CHAR)
605-
return termio_initiate_read(pio);
606-
else
607-
return fileio_ReadFileEx(pio);
608-
609-
return 0;
604+
/* initiate read, record any error so read() will pick up */
605+
if (FILETYPE(pio) == FILE_TYPE_CHAR) {
606+
if (termio_initiate_read(pio) != 0) {
607+
pio->read_details.error = errno;
608+
errno = 0;
609+
return;
610+
}
611+
}
612+
else {
613+
if (fileio_ReadFileEx(pio) != 0) {
614+
pio->read_details.error = errno;
615+
errno = 0;
616+
return;
617+
}
618+
}
610619
}
611620

612621

‎contrib/win32/win32compat/socketio.c

Lines changed: 98 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -98,60 +98,8 @@ socketio_acceptEx(struct w32_io* pio) {
9898
struct acceptEx_context *context;
9999

100100
debug3("acceptEx - io:%p", pio);
101-
if (pio->internal.context == NULL) {
102-
GUID GuidAcceptEx = WSAID_ACCEPTEX;
103-
GUID GuidGetAcceptExSockaddrs = WSAID_GETACCEPTEXSOCKADDRS;
104-
DWORD dwBytes;
105-
106-
context =
107-
(struct acceptEx_context*)malloc(sizeof(struct acceptEx_context));
108-
if (context == NULL) {
109-
errno = ENOMEM;
110-
debug("acceptEx - ERROR:%d, io:%p", errno, pio);
111-
return -1;
112-
}
113-
memset(context, 0, sizeof(struct acceptEx_context));
114-
if (SOCKET_ERROR == WSAIoctl(pio->sock,
115-
SIO_GET_EXTENSION_FUNCTION_POINTER,
116-
&GuidAcceptEx, sizeof(GuidAcceptEx),
117-
&context->lpfnAcceptEx, sizeof(context->lpfnAcceptEx),
118-
&dwBytes, NULL, NULL))
119-
{
120-
free(context);
121-
errno = errno_from_WSALastError();
122-
debug("acceptEx - Ioctl1 ERROR:%d, io:%p", errno, pio);
123-
return -1;
124-
}
125-
126-
if (SOCKET_ERROR == WSAIoctl(pio->sock,
127-
SIO_GET_EXTENSION_FUNCTION_POINTER,
128-
&GuidGetAcceptExSockaddrs, sizeof(GuidGetAcceptExSockaddrs),
129-
&context->lpfnGuidGetAcceptExSockaddrs, sizeof(context->lpfnGuidGetAcceptExSockaddrs),
130-
&dwBytes, NULL, NULL))
131-
{
132-
free(context);
133-
errno = errno_from_WSALastError();
134-
debug("acceptEx - Ioctl2 ERROR:%d, io:%p", errno, pio);
135-
return -1;
136-
}
137-
101+
context = (struct acceptEx_context *)pio->internal.context;
138102

139-
140-
context->accept_socket = INVALID_SOCKET;
141-
pio->internal.context = context;
142-
}
143-
else
144-
context = (struct acceptEx_context *)pio->internal.context;
145-
146-
/* init overlapped event */
147-
if (pio->read_overlapped.hEvent == NULL) {
148-
pio->read_overlapped.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
149-
if ((pio->read_overlapped.hEvent) == NULL) {
150-
errno = ENOMEM;
151-
debug("acceptEx - CreateEvent() ERROR:%d, io:%p", errno, pio);
152-
return -1;
153-
}
154-
}
155103
ResetEvent(pio->read_overlapped.hEvent);
156104

157105
/* create accepting socket */
@@ -257,10 +205,9 @@ socketio_WSARecv(struct w32_io* pio, BOOL* completed) {
257205
pio->read_details.pending = TRUE;
258206
}
259207
else {
260-
/* io has completed due to error, recv() will pick it up */
261-
debug("WSARecv - WSARecv() ERROR:%d io:%p", WSAGetLastError(), pio);
262-
pio->read_details.error = WSAGetLastError();
263-
return 0;
208+
errno = errno_from_WSALastError();
209+
debug("WSARecv - WSARecv() ERROR: io:%p %d", pio, errno);
210+
return -1;
264211
}
265212
}
266213

@@ -334,11 +281,63 @@ socketio_getpeername(struct w32_io* pio, struct sockaddr* name, int* namelen) {
334281
/* implements listen() */
335282
int
336283
socketio_listen(struct w32_io* pio, int backlog) {
284+
struct acceptEx_context* context;
285+
337286
if (SOCKET_ERROR == listen(pio->sock, backlog)) {
338287
errno = errno_from_WSALastError();
339288
debug("listen - listen() ERROR:%d io:%p", errno, pio);
340289
return -1;
341290
}
291+
292+
/* prep for accept*/
293+
{
294+
GUID GuidAcceptEx = WSAID_ACCEPTEX;
295+
GUID GuidGetAcceptExSockaddrs = WSAID_GETACCEPTEXSOCKADDRS;
296+
DWORD dwBytes;
297+
298+
context = (struct acceptEx_context*)malloc(sizeof(struct acceptEx_context));
299+
if (context == NULL) {
300+
errno = ENOMEM;
301+
debug("listen - ERROR:%d, io:%p", errno, pio);
302+
return -1;
303+
}
304+
memset(context, 0, sizeof(struct acceptEx_context));
305+
if (SOCKET_ERROR == WSAIoctl(pio->sock,
306+
SIO_GET_EXTENSION_FUNCTION_POINTER,
307+
&GuidAcceptEx, sizeof(GuidAcceptEx),
308+
&context->lpfnAcceptEx, sizeof(context->lpfnAcceptEx),
309+
&dwBytes, NULL, NULL))
310+
{
311+
free(context);
312+
errno = errno_from_WSALastError();
313+
debug("listen - Ioctl1 ERROR:%d, io:%p", errno, pio);
314+
return -1;
315+
}
316+
317+
if (SOCKET_ERROR == WSAIoctl(pio->sock,
318+
SIO_GET_EXTENSION_FUNCTION_POINTER,
319+
&GuidGetAcceptExSockaddrs, sizeof(GuidGetAcceptExSockaddrs),
320+
&context->lpfnGuidGetAcceptExSockaddrs, sizeof(context->lpfnGuidGetAcceptExSockaddrs),
321+
&dwBytes, NULL, NULL))
322+
{
323+
free(context);
324+
errno = errno_from_WSALastError();
325+
debug("listen - Ioctl2 ERROR:%d, io:%p", errno, pio);
326+
return -1;
327+
}
328+
329+
pio->read_overlapped.hEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
330+
if ((pio->read_overlapped.hEvent) == NULL) {
331+
free(context);
332+
errno = ENOMEM;
333+
debug("listen - CreateEvent() ERROR:%d, io:%p", errno, pio);
334+
return -1;
335+
}
336+
337+
context->accept_socket = INVALID_SOCKET;
338+
pio->internal.context = context;
339+
}
340+
342341
pio->internal.state = SOCK_LISTENING;
343342
return 0;
344343
}
@@ -694,13 +693,9 @@ socketio_accept(struct w32_io* pio, struct sockaddr* addr, int* addrlen) {
694693
if (w32_io_is_blocking(pio)) {
695694
/* block until accept io is complete */
696695
while (FALSE == socketio_is_io_available(pio, TRUE))
697-
{
698-
if (-1 == wait_for_any_event(&pio->read_overlapped.hEvent,
699-
1, INFINITE))
700-
{
696+
if (-1 == wait_for_any_event(&pio->read_overlapped.hEvent,
697+
1, INFINITE))
701698
return NULL;
702-
}
703-
}
704699
}
705700
else {
706701
/* if i/o is not ready */
@@ -911,19 +906,29 @@ socketio_is_io_available(struct w32_io* pio, BOOL rd) {
911906
BOOL pending =
912907
sock_listening ? pio->read_details.pending : pio->write_details.pending;
913908

914-
if (pending && WSAGetOverlappedResult(pio->sock, overlapped,
915-
&numBytes, FALSE, &flags))
916-
return TRUE;
917-
else {
918-
if (pending && WSAGetLastError() != WSA_IO_INCOMPLETE) {
919-
if (sock_listening)
920-
pio->read_details.error = WSAGetLastError();
921-
else
922-
pio->write_details.error = WSAGetLastError();
923-
return TRUE;
909+
if (pending)
910+
/* if there is an error to be picked up */
911+
if (sock_listening) {
912+
if (pio->read_details.error)
913+
return TRUE;
924914
}
925-
return FALSE;
926-
}
915+
else {
916+
if (pio->write_details.error)
917+
return TRUE;
918+
}
919+
920+
if (WSAGetOverlappedResult(pio->sock, overlapped,
921+
&numBytes, FALSE, &flags))
922+
return TRUE;
923+
else if (WSAGetLastError() != WSA_IO_INCOMPLETE) {
924+
if (sock_listening)
925+
pio->read_details.error = WSAGetLastError();
926+
else
927+
pio->write_details.error = WSAGetLastError();
928+
return TRUE;
929+
}
930+
931+
return FALSE;
927932
}
928933
else if (rd) {
929934
if (pio->read_details.remaining || pio->read_details.error)
@@ -937,7 +942,7 @@ socketio_is_io_available(struct w32_io* pio, BOOL rd) {
937942

938943
}
939944
/*start async io (if needed) for accept and recv*/
940-
int
945+
void
941946
socketio_on_select(struct w32_io* pio, BOOL rd) {
942947

943948
enum w32_io_sock_state sock_state = pio->internal.state;
@@ -946,20 +951,29 @@ socketio_on_select(struct w32_io* pio, BOOL rd) {
946951

947952
//nothing to do for writes (that includes connect)
948953
if (!rd)
949-
return 0;
954+
return;
950955

951956
//listening socket - acceptEx if needed
952957
if (sock_state == SOCK_LISTENING) {
953-
if ((!pio->read_details.pending) && (socketio_acceptEx(pio) != 0))
954-
return -1;
958+
if (pio->read_details.pending == FALSE)
959+
if (socketio_acceptEx(pio) != 0) {
960+
/* set error, accept will pick it*/
961+
pio->read_details.error = errno;
962+
errno = 0;
963+
pio->read_details.pending = TRUE;
964+
SetEvent(pio->read_overlapped.hEvent);
965+
return;
966+
}
955967
}
956968
else {
957969
//connected socket - WSARecv if needed
958-
if ((!pio->read_details.pending)
959-
&& (!socketio_is_io_available(pio, rd))
960-
&& (socketio_WSARecv(pio, NULL) != 0))
961-
return -1;
970+
if ((!pio->read_details.pending) && (!socketio_is_io_available(pio, rd)))
971+
if (socketio_WSARecv(pio, NULL) != 0) {
972+
/* set error, recv() will pick it */
973+
pio->read_details.error = errno;
974+
errno = 0;
975+
return;
976+
}
962977
}
963978

964-
return 0;
965979
}

‎contrib/win32/win32compat/termio.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,13 @@ static DWORD WINAPI ReadThread(
4141

4242
if (0 == QueueUserAPC(ReadAPCProc, main_thread, (ULONG_PTR)pio)) {
4343
debug("TermRead thread - ERROR QueueUserAPC failed %d, io:%p", GetLastError(), pio);
44+
pio->read_details.pending = FALSE;
45+
pio->read_details.error = GetLastError();
4446
DebugBreak();
4547
}
4648
return 0;
4749
}
4850

49-
/* TODO - make this a void func*/
5051
int
5152
termio_initiate_read(struct w32_io* pio) {
5253
HANDLE read_thread;
@@ -74,7 +75,6 @@ termio_initiate_read(struct w32_io* pio) {
7475
return 0;
7576
}
7677

77-
/* TODO - make this a void func*/
7878
static VOID CALLBACK WriteAPCProc(
7979
_In_ ULONG_PTR dwParam
8080
) {
@@ -99,11 +99,13 @@ static DWORD WINAPI WriteThread(
9999
if (!WriteFile(WINHANDLE(pio), pio->write_details.buf, write_status.to_transfer,
100100
&write_status.transferred, NULL)) {
101101
write_status.error = GetLastError();
102-
debug("TermWrite thread - ReadFile failed %d, io:%p", GetLastError(), pio);
102+
debug("TermWrite thread - WriteFile failed %d, io:%p", GetLastError(), pio);
103103
}
104104

105105
if (0 == QueueUserAPC(WriteAPCProc, main_thread, (ULONG_PTR)pio)) {
106106
debug("TermWrite thread - ERROR QueueUserAPC failed %d, io:%p", GetLastError(), pio);
107+
pio->write_details.pending = FALSE;
108+
pio->write_details.error = GetLastError();
107109
DebugBreak();
108110
}
109111
return 0;
@@ -124,6 +126,7 @@ termio_initiate_write(struct w32_io* pio, DWORD num_bytes) {
124126
}
125127

126128
pio->write_overlapped.hEvent = write_thread;
129+
pio->write_details.pending = TRUE;
127130
return 0;
128131
}
129132

‎contrib/win32/win32compat/w32fd.c

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -151,13 +151,13 @@ w32_io_is_io_available(struct w32_io* pio, BOOL rd) {
151151
return fileio_is_io_available(pio, rd);
152152
}
153153

154-
int
154+
void
155155
w32_io_on_select(struct w32_io* pio, BOOL rd)
156156
{
157157
if ((pio->type == SOCK_FD))
158-
return socketio_on_select(pio, rd);
159-
160-
return fileio_on_select(pio, rd);
158+
socketio_on_select(pio, rd);
159+
else
160+
fileio_on_select(pio, rd);
161161
}
162162

163163
#define CHECK_FD(fd) do { \
@@ -568,8 +568,7 @@ w32_select(int fds, w32_fd_set* readfds, w32_fd_set* writefds, w32_fd_set* excep
568568
for (int i = 0; i < fds; i++) {
569569

570570
if (readfds && FD_ISSET(i, readfds)) {
571-
if (w32_io_on_select(fd_table.w32_ios[i], TRUE) == -1)
572-
return -1;
571+
w32_io_on_select(fd_table.w32_ios[i], TRUE);
573572
if ((fd_table.w32_ios[i]->type == SOCK_FD)
574573
&& (fd_table.w32_ios[i]->internal.state == SOCK_LISTENING)) {
575574
if (num_events == SELECT_EVENT_LIMIT) {
@@ -582,8 +581,7 @@ w32_select(int fds, w32_fd_set* readfds, w32_fd_set* writefds, w32_fd_set* excep
582581
}
583582

584583
if (writefds && FD_ISSET(i, writefds)) {
585-
if (w32_io_on_select(fd_table.w32_ios[i], FALSE) == -1)
586-
return -1;
584+
w32_io_on_select(fd_table.w32_ios[i], FALSE);
587585
if ((fd_table.w32_ios[i]->type == SOCK_FD)
588586
&& (fd_table.w32_ios[i]->internal.state == SOCK_CONNECTING)) {
589587
if (num_events == SELECT_EVENT_LIMIT) {
@@ -622,7 +620,7 @@ w32_select(int fds, w32_fd_set* readfds, w32_fd_set* writefds, w32_fd_set* excep
622620
/* timeout specified and both fields are 0 - polling mode*/
623621
/* proceed with further wait if not in polling mode*/
624622
if ((timeout == NULL) || (timeout_ms != 0))
625-
/* wait for io if none is already ready */
623+
/* wait for io until any is ready */
626624
while (out_ready_fds == 0) {
627625
ticks_spent = GetTickCount64() - ticks_start;
628626
time_rem = 0;

‎contrib/win32/win32compat/w32fd.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ int wait_for_any_event(HANDLE* events, int num_events, DWORD milli_seconds);
8787
int socketio_initialize();
8888
int socketio_done();
8989
BOOL socketio_is_io_available(struct w32_io* pio, BOOL rd);
90-
int socketio_on_select(struct w32_io* pio, BOOL rd);
90+
void socketio_on_select(struct w32_io* pio, BOOL rd);
9191
struct w32_io* socketio_socket(int domain, int type, int protocol);
9292
struct w32_io* socketio_accept(struct w32_io* pio, struct sockaddr* addr, int* addrlen);
9393
int socketio_setsockopt(struct w32_io* pio, int level, int optname,
@@ -107,7 +107,7 @@ int socketio_close(struct w32_io* pio);
107107

108108
/*POSIX mimic'ing file API*/
109109
BOOL fileio_is_io_available(struct w32_io* pio, BOOL rd);
110-
int fileio_on_select(struct w32_io* pio, BOOL rd);
110+
void fileio_on_select(struct w32_io* pio, BOOL rd);
111111
int fileio_close(struct w32_io* pio);
112112
int fileio_pipe(struct w32_io* pio[2]);
113113
struct w32_io* fileio_open(const char *pathname, int flags, int mode);

0 commit comments

Comments
 (0)