Re: svn commit: r1916243 - /httpd/httpd/trunk/server/mpm/event/event.c
On Tue, Mar 12, 2024 at 4:08 PM Eric Covener wrote: > > below + POD wakeup > > Did not force the path yet where the listener is started (or fold in > the scoreboard change ) +1, maybe ap_queue_term() rather than ap_queue_interrupt_all().
Re: svn commit: r1916243 - /httpd/httpd/trunk/server/mpm/event/event.c
I should add, this against a very divergent $bigco fork so patch may
not work well.
On Tue, Mar 12, 2024 at 11:07 AM Eric Covener wrote:
>
> below + POD wakeup
>
> Did not force the path yet where the listener is started (or fold in
> the scoreboard change )
>
> On Tue, Mar 12, 2024 at 10:54 AM Eric Covener wrote:
> >
> > On Tue, Mar 12, 2024 at 10:30 AM Eric Covener wrote:
> > >
> > > On Tue, Mar 12, 2024 at 10:19 AM Yann Ylavic wrote:
> > > >
> > > > On Tue, Mar 12, 2024 at 3:03 PM Eric Covener wrote:
> > > > >
> > > > > On Tue, Mar 12, 2024 at 8:48 AM Yann Ylavic
> > > > > wrote:
> > > > > >
> > > > > > Maybe it could be:
> > > > > > if (threads_created) {
> > > > >
> > > > > not listener_started?
> > > > >
> > > > > threads_started>0 could just mean we had no scoreboard issues but
> > > > > pthread_create failed on anything but the first thread.
> > > >
> > > > Don't we want the workers to gracefully stop whenever at least one was
> > > > created, or we may deadlock in clean_child_exit()?
> > >
> > > Yes, I see what you mean now. I will try to force some pthread_create
> > > errors w/ the patch soon.
> >
> > It seems like if there is no listener yet, the
> > signal_threads(ST_GRACEFUL) will fail to interrupt the queue (because
> > it defers to the woken up listener)
> > Maybe we do it inline before apr_thread_exit/
> >
> >
> >
> > --
> > Eric Covener
> > cove...@gmail.com
>
>
>
> --
> Eric Covener
> cove...@gmail.com
--
Eric Covener
cove...@gmail.com
Re: svn commit: r1916243 - /httpd/httpd/trunk/server/mpm/event/event.c
below + POD wakeup
Did not force the path yet where the listener is started (or fold in
the scoreboard change )
On Tue, Mar 12, 2024 at 10:54 AM Eric Covener wrote:
>
> On Tue, Mar 12, 2024 at 10:30 AM Eric Covener wrote:
> >
> > On Tue, Mar 12, 2024 at 10:19 AM Yann Ylavic wrote:
> > >
> > > On Tue, Mar 12, 2024 at 3:03 PM Eric Covener wrote:
> > > >
> > > > On Tue, Mar 12, 2024 at 8:48 AM Yann Ylavic
> > > > wrote:
> > > > >
> > > > > Maybe it could be:
> > > > > if (threads_created) {
> > > >
> > > > not listener_started?
> > > >
> > > > threads_started>0 could just mean we had no scoreboard issues but
> > > > pthread_create failed on anything but the first thread.
> > >
> > > Don't we want the workers to gracefully stop whenever at least one was
> > > created, or we may deadlock in clean_child_exit()?
> >
> > Yes, I see what you mean now. I will try to force some pthread_create
> > errors w/ the patch soon.
>
> It seems like if there is no listener yet, the
> signal_threads(ST_GRACEFUL) will fail to interrupt the queue (because
> it defers to the woken up listener)
> Maybe we do it inline before apr_thread_exit/
>
>
>
> --
> Eric Covener
> cove...@gmail.com
--
Eric Covener
cove...@gmail.com
start-threads-2.diff
Description: Binary data
Re: svn commit: r1916243 - /httpd/httpd/trunk/server/mpm/event/event.c
On Tue, Mar 12, 2024 at 10:30 AM Eric Covener wrote:
>
> On Tue, Mar 12, 2024 at 10:19 AM Yann Ylavic wrote:
> >
> > On Tue, Mar 12, 2024 at 3:03 PM Eric Covener wrote:
> > >
> > > On Tue, Mar 12, 2024 at 8:48 AM Yann Ylavic wrote:
> > > >
> > > > Maybe it could be:
> > > > if (threads_created) {
> > >
> > > not listener_started?
> > >
> > > threads_started>0 could just mean we had no scoreboard issues but
> > > pthread_create failed on anything but the first thread.
> >
> > Don't we want the workers to gracefully stop whenever at least one was
> > created, or we may deadlock in clean_child_exit()?
>
> Yes, I see what you mean now. I will try to force some pthread_create
> errors w/ the patch soon.
It seems like if there is no listener yet, the
signal_threads(ST_GRACEFUL) will fail to interrupt the queue (because
it defers to the woken up listener)
Maybe we do it inline before apr_thread_exit/
--
Eric Covener
cove...@gmail.com
Re: svn commit: r1916243 - /httpd/httpd/trunk/server/mpm/event/event.c
On Tue, Mar 12, 2024 at 3:30 PM Eric Covener wrote:
>
> On Tue, Mar 12, 2024 at 10:19 AM Yann Ylavic wrote:
> >
> > On Tue, Mar 12, 2024 at 3:03 PM Eric Covener wrote:
> > >
> > > On Tue, Mar 12, 2024 at 8:48 AM Yann Ylavic wrote:
> > > >
> > > > Maybe it could be:
> > > > if (threads_created) {
> > >
> > > not listener_started?
> > >
> > > threads_started>0 could just mean we had no scoreboard issues but
> > > pthread_create failed on anything but the first thread.
> >
> > Don't we want the workers to gracefully stop whenever at least one was
> > created, or we may deadlock in clean_child_exit()?
>
> Yes, I see what you mean now. I will try to force some pthread_create
> errors w/ the patch soon.
Possibly we want this bit too:
rv = ap_thread_create([i], thread_attr,
worker_thread, my_info, pruntime);
if (rv != APR_SUCCESS) {
+ap_update_child_status_from_indexes(my_child_num, i,
+SERVER_DEAD, NULL);
to restore SERVER_DEAD for the thread we could not create..
Re: svn commit: r1916243 - /httpd/httpd/trunk/server/mpm/event/event.c
On Tue, Mar 12, 2024 at 10:19 AM Yann Ylavic wrote:
>
> On Tue, Mar 12, 2024 at 3:03 PM Eric Covener wrote:
> >
> > On Tue, Mar 12, 2024 at 8:48 AM Yann Ylavic wrote:
> > >
> > > Maybe it could be:
> > > if (threads_created) {
> >
> > not listener_started?
> >
> > threads_started>0 could just mean we had no scoreboard issues but
> > pthread_create failed on anything but the first thread.
>
> Don't we want the workers to gracefully stop whenever at least one was
> created, or we may deadlock in clean_child_exit()?
Yes, I see what you mean now. I will try to force some pthread_create
errors w/ the patch soon.
Re: svn commit: r1916243 - /httpd/httpd/trunk/server/mpm/event/event.c
On Tue, Mar 12, 2024 at 3:03 PM Eric Covener wrote:
>
> On Tue, Mar 12, 2024 at 8:48 AM Yann Ylavic wrote:
> >
> > Maybe it could be:
> > if (threads_created) {
>
> not listener_started?
>
> threads_started>0 could just mean we had no scoreboard issues but
> pthread_create failed on anything but the first thread.
Don't we want the workers to gracefully stop whenever at least one was
created, or we may deadlock in clean_child_exit()?
>
> > resource_shortage = 1;
> > signal_threads(ST_GRACEFUL);
> > break;
>
> I think this would have us still outer looping to keep trying to create
> threads?
Yes, exiting start_threads() is better I think, we should not create
more threads anyway.
>
> > }
> > clean_child_exit(APEXIT_CHILDSICK);
>
> >> We should probably prevent the listener from starting too, like:
>
> Could be confusing, maybe we can instead dodge creating the listener
> if resource_shortage=1?
So something like the attached patch?
Regards;
Yann.
Index: server/mpm/event/event.c
===
--- server/mpm/event/event.c (revision 1916254)
+++ server/mpm/event/event.c (working copy)
@@ -2748,8 +2748,18 @@ static void *APR_THREAD_FUNC start_threads(apr_thr
ap_log_error(APLOG_MARK, APLOG_ALERT, rv, ap_server_conf,
APLOGNO(03104)
"ap_thread_create: unable to create worker thread");
-/* let the parent decide how bad this really is */
-signal_threads(listener_started ? ST_GRACEFUL : ST_UNGRACEFUL);
+/* Let the parent decide how bad this really is by returning
+ * APEXIT_CHILDSICK. If threads were created already let them
+ * stop cleanly first to avoid deadlocks in clean_child_exit(),
+ * just stop creating new ones here (but set resource_shortage
+ * to return APEXIT_CHILDSICK still when the child exists).
+ */
+if (threads_created) {
+resource_shortage = 1;
+signal_threads(ST_GRACEFUL);
+apr_thread_exit(thd, APR_SUCCESS);
+return NULL;
+}
clean_child_exit(APEXIT_CHILDSICK);
}
threads_created++;
Re: svn commit: r1916243 - /httpd/httpd/trunk/server/mpm/event/event.c
On Tue, Mar 12, 2024 at 8:48 AM Yann Ylavic wrote:
>
> On Tue, Mar 12, 2024 at 1:06 PM Eric Covener wrote:
> >
> > On Mon, Mar 11, 2024 at 8:28 PM wrote:
> > >
> > > Author: covener
> > > Date: Tue Mar 12 00:28:34 2024
> > > New Revision: 1916243
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1916243=rev
> > > Log:
> > > use graceful exit if lister started
> > >
> > > Modified:
> > > httpd/httpd/trunk/server/mpm/event/event.c
> > >
> > > Modified: httpd/httpd/trunk/server/mpm/event/event.c
> > > URL:
> > > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1916243=1916242=1916243=diff
> > > ==
> > > --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> > > +++ httpd/httpd/trunk/server/mpm/event/event.c Tue Mar 12 00:28:34 2024
> > > @@ -2749,7 +2749,7 @@ static void *APR_THREAD_FUNC start_threa
> > > APLOGNO(03104)
> > > "ap_thread_create: unable to create worker
> > > thread");
> > > /* let the parent decide how bad this really is */
> > > -signal_threads(ST_UNGRACEFUL);
> > > +signal_threads(listener_started ? ST_GRACEFUL :
> > > ST_UNGRACEFUL);
> > > clean_child_exit(APEXIT_CHILDSICK);
> > > }
> >
> > Maybe this option is silly, if we are going to nearly immediately
> > clear pchild and call exit().
>
> Maybe it could be:
> if (threads_created) {
not listener_started?
threads_started>0 could just mean we had no scoreboard issues but
pthread_create failed on anything but the first thread.
> resource_shortage = 1;
> signal_threads(ST_GRACEFUL);
> break;
I think this would have us still outer looping to keep trying to create threads?
> }
> clean_child_exit(APEXIT_CHILDSICK);
>> We should probably prevent the listener from starting too, like:
Could be confusing, maybe we can instead dodge creating the listener
if resource_shortage=1?
--
Eric Covener
cove...@gmail.com
Re: svn commit: r1916243 - /httpd/httpd/trunk/server/mpm/event/event.c
On Tue, Mar 12, 2024 at 1:46 PM Yann Ylavic wrote:
>
> Maybe it could be:
We should probably prevent the listener from starting too, like:
> if (threads_created) {
> resource_shortage = 1;
> signal_threads(ST_GRACEFUL);
listener_started = 1; /* don't start it if not already */
> break;
> }
> clean_child_exit(APEXIT_CHILDSICK);
> ?
>
>
> Regards;
> Yann.
Re: svn commit: r1916243 - /httpd/httpd/trunk/server/mpm/event/event.c
On Tue, Mar 12, 2024 at 1:06 PM Eric Covener wrote:
>
> On Mon, Mar 11, 2024 at 8:28 PM wrote:
> >
> > Author: covener
> > Date: Tue Mar 12 00:28:34 2024
> > New Revision: 1916243
> >
> > URL: http://svn.apache.org/viewvc?rev=1916243=rev
> > Log:
> > use graceful exit if lister started
> >
> > Modified:
> > httpd/httpd/trunk/server/mpm/event/event.c
> >
> > Modified: httpd/httpd/trunk/server/mpm/event/event.c
> > URL:
> > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1916243=1916242=1916243=diff
> > ==
> > --- httpd/httpd/trunk/server/mpm/event/event.c (original)
> > +++ httpd/httpd/trunk/server/mpm/event/event.c Tue Mar 12 00:28:34 2024
> > @@ -2749,7 +2749,7 @@ static void *APR_THREAD_FUNC start_threa
> > APLOGNO(03104)
> > "ap_thread_create: unable to create worker
> > thread");
> > /* let the parent decide how bad this really is */
> > -signal_threads(ST_UNGRACEFUL);
> > +signal_threads(listener_started ? ST_GRACEFUL :
> > ST_UNGRACEFUL);
> > clean_child_exit(APEXIT_CHILDSICK);
> > }
>
> Maybe this option is silly, if we are going to nearly immediately
> clear pchild and call exit().
Maybe it could be:
if (threads_created) {
resource_shortage = 1;
signal_threads(ST_GRACEFUL);
break;
}
clean_child_exit(APEXIT_CHILDSICK);
?
Regards;
Yann.
Re: svn commit: r1916243 - /httpd/httpd/trunk/server/mpm/event/event.c
On Mon, Mar 11, 2024 at 8:28 PM wrote: > > Author: covener > Date: Tue Mar 12 00:28:34 2024 > New Revision: 1916243 > > URL: http://svn.apache.org/viewvc?rev=1916243=rev > Log: > use graceful exit if lister started > > Modified: > httpd/httpd/trunk/server/mpm/event/event.c > > Modified: httpd/httpd/trunk/server/mpm/event/event.c > URL: > http://svn.apache.org/viewvc/httpd/httpd/trunk/server/mpm/event/event.c?rev=1916243=1916242=1916243=diff > == > --- httpd/httpd/trunk/server/mpm/event/event.c (original) > +++ httpd/httpd/trunk/server/mpm/event/event.c Tue Mar 12 00:28:34 2024 > @@ -2749,7 +2749,7 @@ static void *APR_THREAD_FUNC start_threa > APLOGNO(03104) > "ap_thread_create: unable to create worker > thread"); > /* let the parent decide how bad this really is */ > -signal_threads(ST_UNGRACEFUL); > +signal_threads(listener_started ? ST_GRACEFUL : > ST_UNGRACEFUL); > clean_child_exit(APEXIT_CHILDSICK); > } Maybe this option is silly, if we are going to nearly immediately clear pchild and call exit(). -- Eric Covener cove...@gmail.com
