You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Yann Ylavic <yl...@gmail.com> on 2022/01/19 21:59:08 UTC
Re: svn commit: r1897208 - in /apr/apr/branches/win32-pollset-wakeup-no-file-socket-emulation: file_io/win32/ include/arch/unix/ include/arch/win32/ poll/unix/
On Wed, Jan 19, 2022 at 5:42 PM <iv...@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,
--
Regards;
Yann.
Re: svn commit: r1897208 - in /apr/apr/branches/win32-pollset-wakeup-no-file-socket-emulation: file_io/win32/ include/arch/unix/ include/arch/win32/ poll/unix/
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Thu, 20 Jan 2022 at 01:00, Yann Ylavic <yl...@gmail.com> wrote:
> On Wed, Jan 19, 2022 at 5:42 PM <iv...@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