https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=7c963c7ba030b9e110eefd6412eff4d6189f29e7
commit 7c963c7ba030b9e110eefd6412eff4d6189f29e7 Author: Corinna Vinschen <[email protected]> Date: Fri Aug 28 15:22:58 2020 +0200 Cygwin: sigproc: Allow more child processes per process 256 children per process is a bit tight in some scenarios. Fix this by revamping the `procs' array. Convert it to an extensible class child_procs and rename procs to chld_procs. Fix code throughout to use matching class methods rather than direct access. To allow a lot more child processes while trying to avoid allocations at DLL startup, maintain two arrays within class child_procs, one using a default size for 255 (i686) or 1023 (x86_64) children, the other, dynamically allocated on overflowing the first array, giving room for another 1023 (i686) or 4095 (x86_64) processes. On testing with a simple reproducer on a x86_64 machine with 4 Gigs RAM, a system memory overflow occured after forking about 1450 child processes, so this simple dynamic should suffice for a while. Signed-off-by: Corinna Vinschen <[email protected]> Diff: --- winsup/cygwin/child_info.h | 2 - winsup/cygwin/sigproc.cc | 145 +++++++++++++++++++++++++++++---------------- 2 files changed, 93 insertions(+), 54 deletions(-) diff --git a/winsup/cygwin/child_info.h b/winsup/cygwin/child_info.h index e5aa28144..505eaef23 100644 --- a/winsup/cygwin/child_info.h +++ b/winsup/cygwin/child_info.h @@ -39,8 +39,6 @@ enum child_status /* Change this value if you get a message indicating that it is out-of-sync. */ #define CURR_CHILD_INFO_MAGIC 0xecc930b9U -#define NPROCS 256 - #include "pinfo.h" struct cchildren { diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc index ba1e64b33..43da97bc3 100644 --- a/winsup/cygwin/sigproc.cc +++ b/winsup/cygwin/sigproc.cc @@ -38,19 +38,55 @@ int __sp_ln; bool no_thread_exit_protect::flag; -char NO_COPY myself_nowait_dummy[1] = {'0'};// Flag to sig_send that signal goes to - // current process but no wait is required +/* Flag to sig_send that signal goes to current process but no wait is + required. */ +char NO_COPY myself_nowait_dummy[1] = {'0'}; #define Static static NO_COPY +/* All my children info. Avoid expensive constructor ops at DLL startup */ +class child_procs { +#ifdef __i386__ + static const int _NPROCS = 255; + static const int _NPROCS_2 = 1023; +#else + static const int _NPROCS = 1023; + static const int _NPROCS_2 = 4095; +#endif + int _count; + uint8_t _procs[(_NPROCS + 1) * sizeof (pinfo)] __attribute__ ((__aligned__)); + pinfo *_procs_2; + public: + int count () const { return _count; } + int add_one () { return ++_count; } + int del_one () { return --_count; } + int reset () { return _count = 0; } + pinfo &operator[] (int idx) + { + if (idx >= _NPROCS) + { + if (!_procs_2) + { + /* Use HeapAlloc to avoid propagating this memory area + to the child processes. */ + _procs_2 = (pinfo *) HeapAlloc (GetProcessHeap (), + HEAP_GENERATE_EXCEPTIONS + | HEAP_ZERO_MEMORY, + (_NPROCS_2 + 1) * sizeof (pinfo)); + } + return _procs_2[idx - _NPROCS]; + } + return ((pinfo *) _procs)[idx]; + } + int max_child_procs () const { return _NPROCS + _NPROCS_2; } +}; +Static child_procs chld_procs; -Static int nprocs; // Number of deceased children -Static char cprocs[(NPROCS + 1) * sizeof (pinfo)];// All my children info -#define procs ((pinfo *) cprocs) // All this just to avoid expensive - // constructor operation at DLL startup -Static waitq waitq_head; // Start of queue for wait'ing threads +/* Start of queue for waiting threads. */ +Static waitq waitq_head; -Static muto sync_proc_subproc; // Control access to subproc stuff +/* Controls access to subproc stuff. */ +Static muto sync_proc_subproc; _cygtls NO_COPY *_sig_tls; @@ -163,8 +199,8 @@ pid_exists (pid_t pid) static inline bool mychild (int pid) { - for (int i = 0; i < nprocs; i++) - if (procs[i]->pid == pid) + for (int i = 0; i < chld_procs.count (); i++) + if (chld_procs[i]->pid == pid) return true; return false; } @@ -174,6 +210,7 @@ mychild (int pid) int __reg2 proc_subproc (DWORD what, uintptr_t val) { + int slot; int rc = 1; int potential_match; int clearing; @@ -197,14 +234,15 @@ proc_subproc (DWORD what, uintptr_t val) */ case PROC_ADD_CHILD: /* Filled up process table? */ - if (nprocs >= NPROCS) + if (chld_procs.count () >= chld_procs.max_child_procs ()) { sigproc_printf ("proc table overflow: hit %d processes, pid %d\n", - nprocs, vchild->pid); + chld_procs.count (), vchild->pid); rc = 0; set_errno (EAGAIN); break; } + if (vchild != myself) { vchild->uid = myself->uid; @@ -219,13 +257,14 @@ proc_subproc (DWORD what, uintptr_t val) break; case PROC_ATTACH_CHILD: - procs[nprocs] = vchild; - rc = procs[nprocs].wait (); + slot = chld_procs.count (); + chld_procs[slot] = vchild; + rc = chld_procs[slot].wait (); if (rc) { sigproc_printf ("added pid %d to proc table, slot %d", vchild->pid, - nprocs); - nprocs++; + slot); + chld_procs.add_one (); } break; @@ -260,7 +299,7 @@ proc_subproc (DWORD what, uintptr_t val) goto scan_wait; case PROC_EXEC_CLEANUP: - while (nprocs) + while (chld_procs.count ()) remove_proc (0); for (w = &waitq_head; w->next != NULL; w = w->next) CloseHandle (w->next->ev); @@ -274,7 +313,8 @@ proc_subproc (DWORD what, uintptr_t val) if (val) sigproc_printf ("clear waiting threads"); else - sigproc_printf ("looking for processes to reap, nprocs %d", nprocs); + sigproc_printf ("looking for processes to reap, count %d", + chld_procs.count ()); clearing = val; scan_wait: @@ -312,7 +352,7 @@ proc_subproc (DWORD what, uintptr_t val) } if (global_sigs[SIGCHLD].sa_handler == (void *) SIG_IGN) - for (int i = 0; i < nprocs; i += remove_proc (i)) + for (int i = 0; i < chld_procs.count (); i += remove_proc (i)) continue; } @@ -352,35 +392,35 @@ _cygtls::remove_wq (DWORD wait) Called on process exit. Also called by spawn_guts to disassociate any subprocesses from this process. Subprocesses will then know to clean up after themselves and - will not become procs. */ + will not become chld_procs. */ void proc_terminate () { - sigproc_printf ("nprocs %d", nprocs); - if (nprocs) + sigproc_printf ("child_procs count %d", chld_procs.count ()); + if (chld_procs.count ()) { sync_proc_subproc.acquire (WPSP); proc_subproc (PROC_CLEARWAIT, 1); /* Clean out proc processes from the pid list. */ - for (int i = 0; i < nprocs; i++) + for (int i = 0; i < chld_procs.count (); i++) { /* If we've execed then the execed process will handle setting ppid to 1 iff it is a Cygwin process. */ if (!have_execed || !have_execed_cygwin) - procs[i]->ppid = 1; - if (procs[i].wait_thread) - procs[i].wait_thread->terminate_thread (); + chld_procs[i]->ppid = 1; + if (chld_procs[i].wait_thread) + chld_procs[i].wait_thread->terminate_thread (); /* Release memory associated with this process unless it is 'myself'. - 'myself' is only in the procs table when we've execed. We reach - here when the next process has finished initializing but we still - can't free the memory used by 'myself' since it is used later on - during cygwin tear down. */ - if (procs[i] != myself) - procs[i].release (); + 'myself' is only in the chld_procs table when we've execed. We + reach here when the next process has finished initializing but we + still can't free the memory used by 'myself' since it is used + later on during cygwin tear down. */ + if (chld_procs[i] != myself) + chld_procs[i].release (); } - nprocs = 0; + chld_procs.reset (); sync_proc_subproc.release (); } sigproc_printf ("leaving"); @@ -866,7 +906,8 @@ cygheap_exec_info::alloc () cygheap_exec_info *res = (cygheap_exec_info *) ccalloc_abort (HEAP_1_EXEC, 1, sizeof (cygheap_exec_info) - + (nprocs * sizeof (children[0]))); + + (chld_procs.count () + * sizeof (children[0]))); res->sigmask = _my_tls.sigmask; return res; } @@ -942,10 +983,10 @@ child_info_spawn::cleanup () inline void cygheap_exec_info::record_children () { - for (nchildren = 0; nchildren < nprocs; nchildren++) + for (nchildren = 0; nchildren < chld_procs.count (); nchildren++) { - children[nchildren].pid = procs[nchildren]->pid; - children[nchildren].p = procs[nchildren]; + children[nchildren].pid = chld_procs[nchildren]->pid; + children[nchildren].p = chld_procs[nchildren]; /* Set inheritance of required child handles for reattach_children in the about-to-be-execed process. */ children[nchildren].p.set_inheritance (true); @@ -1126,13 +1167,13 @@ checkstate (waitq *parent_w) { int potential_match = 0; - sigproc_printf ("nprocs %d", nprocs); + sigproc_printf ("child_procs count %d", chld_procs.count ()); /* Check already dead processes first to see if they match the criteria * given in w->next. */ int res; - for (int i = 0; i < nprocs; i++) - if ((res = stopped_or_terminated (parent_w, procs[i]))) + for (int i = 0; i < chld_procs.count (); i++) + if ((res = stopped_or_terminated (parent_w, chld_procs[i]))) { remove_proc (i); potential_match = 1; @@ -1140,40 +1181,40 @@ checkstate (waitq *parent_w) } sigproc_printf ("no matching terminated children found"); - potential_match = -!!nprocs; + potential_match = -!!chld_procs.count (); out: sigproc_printf ("returning %d", potential_match); return potential_match; } -/* Remove a proc from procs by swapping it with the last child in the list. +/* Remove a proc from chld_procs by swapping it with the last child in the list. Also releases shared memory of exited processes. */ static int remove_proc (int ci) { if (have_execed) { - if (_my_tls._ctinfo != procs[ci].wait_thread) - procs[ci].wait_thread->terminate_thread (); + if (_my_tls._ctinfo != chld_procs[ci].wait_thread) + chld_procs[ci].wait_thread->terminate_thread (); } - else if (procs[ci] && procs[ci]->exists ()) + else if (chld_procs[ci] && chld_procs[ci]->exists ()) return 1; - sigproc_printf ("removing procs[%d], pid %d, nprocs %d", ci, procs[ci]->pid, - nprocs); - if (procs[ci] != myself) - procs[ci].release (); - if (ci < --nprocs) + sigproc_printf ("removing chld_procs[%d], pid %d, child_procs count %d", + ci, chld_procs[ci]->pid, chld_procs.count ()); + if (chld_procs[ci] != myself) + chld_procs[ci].release (); + if (ci < chld_procs.del_one ()) { /* Wait for proc_waiter thread to make a copy of this element before moving it or it may become confused. The chances are very high that the proc_waiter thread has already done this by the time we get here. */ if (!have_execed && !exit_state) - while (!procs[nprocs].waiter_ready) + while (!chld_procs[chld_procs.count ()].waiter_ready) yield (); - procs[ci] = procs[nprocs]; + chld_procs[ci] = chld_procs[chld_procs.count ()]; } return 0; }
