You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rb...@locus.apache.org on 2000/06/22 21:06:08 UTC
cvs commit: apache-2.0/src/modules/mpm/dexter dexter.c
rbb 00/06/22 12:06:08
Modified: src/modules/mpm/dexter dexter.c
Log:
Make dexter use the ap_poll API. This change is forced by AIX's
redefinition of pollfd.
Revision Changes Path
1.100 +23 -19 apache-2.0/src/modules/mpm/dexter/dexter.c
Index: dexter.c
===================================================================
RCS file: /home/cvs/apache-2.0/src/modules/mpm/dexter/dexter.c,v
retrieving revision 1.99
retrieving revision 1.100
diff -u -r1.99 -r1.100
--- dexter.c 2000/06/22 00:28:32 1.99
+++ dexter.c 2000/06/22 19:06:07 1.100
@@ -104,7 +104,7 @@
static int workers_may_exit = 0;
static int requests_this_child;
static int num_listenfds = 0;
-static struct pollfd *listenfds;
+static ap_socket_t **listenfds;
struct ap_ctable ap_child_table[HARD_SERVER_LIMIT];
@@ -572,9 +572,10 @@
if (!workers_may_exit) {
int ret;
char pipe_read_char;
+ ap_ssize_t n = 1;
- ret = read(listenfds[0].fd, &pipe_read_char, 1);
- if (ret == -1 && (errno == EAGAIN || errno == EWOULDBLOCK)) {
+ ret = ap_recv(listenfds[0], &pipe_read_char, &n);
+ if (ap_canonical_error(ret) == APR_EAGAIN) {
/* It lost the lottery. It must continue to suffer
* through a life of servitude. */
}
@@ -600,6 +601,8 @@
int thread_just_started = 1;
int thread_num = *((int *) arg);
long conn_id = child_num * HARD_THREAD_LIMIT + thread_num;
+ ap_pollfd_t *pollset;
+ int n;
ap_status_t rv;
pthread_mutex_lock(&thread_pool_parent_mutex);
@@ -607,6 +610,10 @@
pthread_mutex_unlock(&thread_pool_parent_mutex);
ap_create_pool(&ptrans, tpool);
+ ap_setup_poll(&pollset, num_listenfds+1, tpool);
+ for(n=0 ; n <= num_listenfds ; ++n)
+ ap_add_poll_socket(pollset, listenfds[n], APR_POLLIN);
+
while (!workers_may_exit) {
workers_may_exit |= (max_requests_per_child != 0) && (requests_this_child <= 0);
if (workers_may_exit) break;
@@ -638,10 +645,11 @@
}
while (!workers_may_exit) {
- srv = poll(listenfds, num_listenfds + 1, -1);
+ ap_int16_t event;
+ srv = ap_poll(pollset, &n, -1);
- if (srv < 0) {
- if (errno == EINTR) {
+ if (srv != APR_SUCCESS) {
+ if (errno == APR_EINTR) {
continue;
}
@@ -653,7 +661,8 @@
}
if (workers_may_exit) break;
- if (listenfds[0].revents & POLLIN) {
+ ap_get_revents(&event, listenfds[0], pollset);
+ if (event & APR_POLLIN) {
/* A process got a signal on the shutdown pipe. Check if we're
* the lucky process to die. */
check_pipe_of_death();
@@ -673,10 +682,10 @@
curr_pollfd = 1;
}
/* XXX: Should we check for POLLERR? */
- if (listenfds[curr_pollfd].revents & POLLIN) {
+ ap_get_revents(&event, listenfds[curr_pollfd], pollset);
+ if (event & APR_POLLIN) {
last_pollfd = curr_pollfd;
- sd = NULL;
- ap_put_os_sock(&sd, &listenfds[curr_pollfd].fd, ptrans);
+ sd = listenfds[curr_pollfd];
goto got_fd;
}
} while (curr_pollfd != last_pollfd);
@@ -787,15 +796,10 @@
requests_this_child = max_requests_per_child;
/* Set up the pollfd array */
- listenfds = ap_palloc(pchild, sizeof(struct pollfd) * (num_listenfds + 1));
- listenfds[0].fd = pipe_of_death[0];
- listenfds[0].events = POLLIN;
- listenfds[0].revents = 0;
- for (lr = ap_listeners, i = 1; i <= num_listenfds; lr = lr->next, ++i) {
- ap_get_os_sock(&listenfds[i].fd, lr->sd);
- listenfds[i].events = POLLIN; /* should we add POLLPRI ?*/
- listenfds[i].revents = 0;
- }
+ listenfds = ap_palloc(pchild, sizeof(*listenfds) * (num_listenfds + 1));
+ ap_put_os_sock(&listenfds[0], &pipe_of_death[0], pchild);
+ for (lr = ap_listeners, i = 1; i <= num_listenfds; lr = lr->next, ++i)
+ listenfds[i]=lr->sd;
/* Setup worker threads */
Re: cvs commit: apache-2.0/src/modules/mpm/dexter dexter.c
Posted by rb...@covalent.net.
> > while (!workers_may_exit) {
> > - srv = poll(listenfds, num_listenfds + 1, -1);
> > + ap_int16_t event;
> > + srv = ap_poll(pollset, &n, -1);
> >
> > - if (srv < 0) {
> > - if (errno == EINTR) {
> > + if (srv != APR_SUCCESS) {
> > + if (errno == APR_EINTR) {
>
> Shouldn't we be checking srv instead of errno? Should we
> cannonicalize it before comparing with APR_EINTR?
Yes. This code was taken form mpmt_pthread, so I guess it's broken there
too. I'll fix both in a few minutes.
>
> > @@ -787,15 +796,10 @@
> > requests_this_child = max_requests_per_child;
> >
> > /* Set up the pollfd array */
> > - listenfds = ap_palloc(pchild, sizeof(struct pollfd) * (num_listenfds + 1));
> > - listenfds[0].fd = pipe_of_death[0];
> > - listenfds[0].events = POLLIN;
> > - listenfds[0].revents = 0;
> > - for (lr = ap_listeners, i = 1; i <= num_listenfds; lr = lr->next, ++i) {
> > - ap_get_os_sock(&listenfds[i].fd, lr->sd);
> > - listenfds[i].events = POLLIN; /* should we add POLLPRI ?*/
> > - listenfds[i].revents = 0;
> > - }
> > + listenfds = ap_palloc(pchild, sizeof(*listenfds) * (num_listenfds + 1));
> > + ap_put_os_sock(&listenfds[0], &pipe_of_death[0], pchild);
>
> If we don't set the storage pointed to by listenfds to zero (via
> ap_pcalloc()), won't ap_put_os_sock() assume that listenfds[n] is a pointer to
> existing storage to be used for the ap_sock_t, rather than a place to
> store the pointer to new storage to be used for the ap_sock_t?
> Instead, listenfds[0] is uninitialized.
Yep. I need to fix this too. I was fixing some code that was in
mpmt_pthread at the time and not looking very hard at the code. IT didn't
help that I was thinking of the massage I was about to get. I'll fix this
as soon as I make one other commit.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: apache-2.0/src/modules/mpm/dexter dexter.c
Posted by rb...@covalent.net.
> > while (!workers_may_exit) {
> > - srv = poll(listenfds, num_listenfds + 1, -1);
> > + ap_int16_t event;
> > + srv = ap_poll(pollset, &n, -1);
> >
> > - if (srv < 0) {
> > - if (errno == EINTR) {
> > + if (srv != APR_SUCCESS) {
> > + if (errno == APR_EINTR) {
>
> Shouldn't we be checking srv instead of errno? Should we
> cannonicalize it before comparing with APR_EINTR?
Yes. This code was taken form mpmt_pthread, so I guess it's broken there
too. I'll fix both in a few minutes.
>
> > @@ -787,15 +796,10 @@
> > requests_this_child = max_requests_per_child;
> >
> > /* Set up the pollfd array */
> > - listenfds = ap_palloc(pchild, sizeof(struct pollfd) * (num_listenfds + 1));
> > - listenfds[0].fd = pipe_of_death[0];
> > - listenfds[0].events = POLLIN;
> > - listenfds[0].revents = 0;
> > - for (lr = ap_listeners, i = 1; i <= num_listenfds; lr = lr->next, ++i) {
> > - ap_get_os_sock(&listenfds[i].fd, lr->sd);
> > - listenfds[i].events = POLLIN; /* should we add POLLPRI ?*/
> > - listenfds[i].revents = 0;
> > - }
> > + listenfds = ap_palloc(pchild, sizeof(*listenfds) * (num_listenfds + 1));
> > + ap_put_os_sock(&listenfds[0], &pipe_of_death[0], pchild);
>
> If we don't set the storage pointed to by listenfds to zero (via
> ap_pcalloc()), won't ap_put_os_sock() assume that listenfds[n] is a pointer to
> existing storage to be used for the ap_sock_t, rather than a place to
> store the pointer to new storage to be used for the ap_sock_t?
> Instead, listenfds[0] is uninitialized.
Yep. I need to fix this too. I was fixing some code that was in
mpmt_pthread at the time and not looking very hard at the code. IT didn't
help that I was thinking of the massage I was about to get. I'll fix this
as soon as I make one other commit.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------
Re: cvs commit: apache-2.0/src/modules/mpm/dexter dexter.c
Posted by Jeff Trawick <tr...@ibm.net>.
> From: rbb@locus.apache.org
> Date: 22 Jun 2000 19:06:08 -0000
>
> Index: dexter.c
> ===================================================================
> RCS file: /home/cvs/apache-2.0/src/modules/mpm/dexter/dexter.c,v
...
> while (!workers_may_exit) {
> - srv = poll(listenfds, num_listenfds + 1, -1);
> + ap_int16_t event;
> + srv = ap_poll(pollset, &n, -1);
>
> - if (srv < 0) {
> - if (errno == EINTR) {
> + if (srv != APR_SUCCESS) {
> + if (errno == APR_EINTR) {
Shouldn't we be checking srv instead of errno? Should we
cannonicalize it before comparing with APR_EINTR?
> @@ -787,15 +796,10 @@
> requests_this_child = max_requests_per_child;
>
> /* Set up the pollfd array */
> - listenfds = ap_palloc(pchild, sizeof(struct pollfd) * (num_listenfds + 1));
> - listenfds[0].fd = pipe_of_death[0];
> - listenfds[0].events = POLLIN;
> - listenfds[0].revents = 0;
> - for (lr = ap_listeners, i = 1; i <= num_listenfds; lr = lr->next, ++i) {
> - ap_get_os_sock(&listenfds[i].fd, lr->sd);
> - listenfds[i].events = POLLIN; /* should we add POLLPRI ?*/
> - listenfds[i].revents = 0;
> - }
> + listenfds = ap_palloc(pchild, sizeof(*listenfds) * (num_listenfds + 1));
> + ap_put_os_sock(&listenfds[0], &pipe_of_death[0], pchild);
If we don't set the storage pointed to by listenfds to zero (via
ap_pcalloc()), won't ap_put_os_sock() assume that listenfds[n] is a pointer to
existing storage to be used for the ap_sock_t, rather than a place to
store the pointer to new storage to be used for the ap_sock_t?
Instead, listenfds[0] is uninitialized.
> + for (lr = ap_listeners, i = 1; i <= num_listenfds; lr = lr->next, ++i)
> + listenfds[i]=lr->sd;
--
Jeff Trawick | trawick@ibm.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...