On Tue, Mar 12, 2024 at 3:03 PM Eric Covener <cove...@gmail.com> wrote:
>
> On Tue, Mar 12, 2024 at 8:48 AM Yann Ylavic <ylavic....@gmail.com> 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++;
