On Thu, 20 Jan 2022 at 01:00, Yann Ylavic <ylavic....@gmail.com> wrote:
> On Wed, Jan 19, 2022 at 5:42 PM <i...@apache.org> wrote: > > > > Author: ivan > > Date: Wed Jan 19 16:41:59 2022 > > New Revision: 1897208 > > > > URL: http://svn.apache.org/viewvc?rev=1897208&view=rev > > Log: > > On 'win32-pollset-wakeup-no-file-socket-emulation' branch: > > > > Windows: For the pollset wakeup, use apr_socket_t directly instead of > using a > > socket disguised as an apr_file_t. > [] > > > > -apr_status_t apr_file_socket_pipe_create(apr_file_t **in, > > - apr_file_t **out, > > +apr_status_t apr_file_socket_pipe_create(apr_socket_t **in, > > + apr_socket_t **out, > > apr_pool_t *p) > > { > > apr_status_t rv; > > SOCKET rd; > > SOCKET wr; > > > > + *in = NULL; > > + *out = NULL; > > + > > if ((rv = create_socket_pipe(&rd, &wr)) != APR_SUCCESS) { > > return rv; > > } > > - (*in) = (apr_file_t *)apr_pcalloc(p, sizeof(apr_file_t)); > > - (*in)->pool = p; > > - (*in)->fname = NULL; > > - (*in)->ftype = APR_FILETYPE_SOCKET; > > - (*in)->timeout = 0; /* read end of the pipe is non-blocking */ > > - (*in)->ungetchar = -1; > > - (*in)->eof_hit = 0; > > - (*in)->filePtr = 0; > > - (*in)->bufpos = 0; > > - (*in)->dataRead = 0; > > - (*in)->direction = 0; > > - (*in)->pOverlapped = NULL; > > - (*in)->filehand = (HANDLE)rd; > > - > > - (*out) = (apr_file_t *)apr_pcalloc(p, sizeof(apr_file_t)); > > - (*out)->pool = p; > > - (*out)->fname = NULL; > > - (*out)->ftype = APR_FILETYPE_SOCKET; > > - (*out)->timeout = -1; > > - (*out)->ungetchar = -1; > > - (*out)->eof_hit = 0; > > - (*out)->filePtr = 0; > > - (*out)->bufpos = 0; > > - (*out)->dataRead = 0; > > - (*out)->direction = 0; > > - (*out)->pOverlapped = NULL; > > - (*out)->filehand = (HANDLE)wr; > > + apr_os_sock_put(in, &rd, p); > > + apr_os_sock_put(out, &wr, p); > > I think that the *in apr_socket and the underlying socket descriptor > are not aligned w.r.t. non-blocking. > We probably need to call apr_socket_timeout_set(*in, 0) here to make > that happen, thus remove the last ioctlsocket() in > create_socket_pipe() that sets the descriptor non-blocking before > leaving (which would be useless now). > > Something like the below on top of your branch? > > Index: file_io/win32/pipe.c > =================================================================== > --- file_io/win32/pipe.c (revision 1897212) > +++ file_io/win32/pipe.c (working copy) > @@ -381,14 +381,7 @@ static apr_status_t create_socket_pipe(SOCKET *rd, > goto cleanup; > } > if (nrd == (int)sizeof(uid) && memcmp(iid, uid, sizeof(uid)) == > 0) { > - /* Got the right identifier, put the poll()able read side of > - * the pipe in nonblocking mode and return. > - */ > - bm = 1; > - if (ioctlsocket(*rd, FIONBIO, &bm) == SOCKET_ERROR) { > - rv = apr_get_netos_error(); > - goto cleanup; > - } > + /* Got the right identifier, return. */ > break; > } > closesocket(*rd); > @@ -438,6 +431,9 @@ apr_status_t apr_file_socket_pipe_create(apr_socke > apr_os_sock_put(in, &rd, p); > apr_os_sock_put(out, &wr, p); > > + /* read end of the pipe is non-blocking */ > + apr_socket_timeout_set(*in, 0); > + > apr_pool_cleanup_register(p, (void *)(*in), socket_pipe_cleanup, > apr_pool_cleanup_null); > apr_pool_cleanup_register(p, (void *)(*out), socket_pipe_cleanup, > -- > > Makes sense to me. Committed in r1897245 . Thanks! -- Ivan Zhakov