Author: vlendec Date: 2006-08-16 10:36:19 +0000 (Wed, 16 Aug 2006) New Revision: 17569
WebSVN: http://websvn.samba.org/cgi-bin/viewcvs.cgi?view=rev&root=samba&rev=17569 Log: Make 'max smbd processes' more robust. Counting on the child to decrement a tdb entry is not the most reliable way to count children correctly. This increments the number of children after a fork and decrements it upon SIGCLD. I'm keeping a list of children just for consistency checks, so that we at least get a debug level 0 message if something goes wrong. Volker Modified: branches/SAMBA_3_0/source/lib/dummysmbd.c branches/SAMBA_3_0/source/lib/util.c branches/SAMBA_3_0/source/smbd/process.c branches/SAMBA_3_0/source/smbd/server.c Changeset: Modified: branches/SAMBA_3_0/source/lib/dummysmbd.c =================================================================== --- branches/SAMBA_3_0/source/lib/dummysmbd.c 2006-08-16 09:10:54 UTC (rev 17568) +++ branches/SAMBA_3_0/source/lib/dummysmbd.c 2006-08-16 10:36:19 UTC (rev 17569) @@ -24,11 +24,6 @@ #include "includes.h" -void decrement_smbd_process_count( void ) -{ - return; -} - int find_service(fstring service) { return -1; Modified: branches/SAMBA_3_0/source/lib/util.c =================================================================== --- branches/SAMBA_3_0/source/lib/util.c 2006-08-16 09:10:54 UTC (rev 17568) +++ branches/SAMBA_3_0/source/lib/util.c 2006-08-16 10:36:19 UTC (rev 17569) @@ -1594,9 +1594,6 @@ (unsigned long long)sys_getpid(), why)); log_stack_trace(); - /* only smbd needs to decrement the smbd counter in connections.tdb */ - decrement_smbd_process_count(); - cmd = lp_panic_action(); if (cmd && *cmd) { DEBUG(0, ("smb_panic(): calling panic action [%s]\n", cmd)); Modified: branches/SAMBA_3_0/source/smbd/process.c =================================================================== --- branches/SAMBA_3_0/source/smbd/process.c 2006-08-16 09:10:54 UTC (rev 17568) +++ branches/SAMBA_3_0/source/smbd/process.c 2006-08-16 10:36:19 UTC (rev 17569) @@ -1032,60 +1032,6 @@ } /**************************************************************************** - Keep track of the number of running smbd's. This functionality is used to - 'hard' limit Samba overhead on resource constrained systems. -****************************************************************************/ - -static BOOL process_count_update_successful = False; - -static int32 increment_smbd_process_count(void) -{ - int32 total_smbds; - - if (lp_max_smbd_processes()) { - total_smbds = 0; - if (tdb_change_int32_atomic(conn_tdb_ctx(), "INFO/total_smbds", &total_smbds, 1) == -1) - return 1; - process_count_update_successful = True; - return total_smbds + 1; - } - return 1; -} - -void decrement_smbd_process_count(void) -{ - int32 total_smbds; - - if (lp_max_smbd_processes() && process_count_update_successful) { - total_smbds = 1; - tdb_change_int32_atomic(conn_tdb_ctx(), "INFO/total_smbds", &total_smbds, -1); - } -} - -static BOOL smbd_process_limit(void) -{ - int32 total_smbds; - - if (lp_max_smbd_processes()) { - - /* Always add one to the smbd process count, as exit_server() always - * subtracts one. - */ - - if (!conn_tdb_ctx()) { - DEBUG(0,("smbd_process_limit: max smbd processes parameter set with status parameter not \ -set. Ignoring max smbd restriction.\n")); - return False; - } - - total_smbds = increment_smbd_process_count(); - return total_smbds > lp_max_smbd_processes(); - } - else - return False; -} - -/**************************************************************************** Process an smb from the client ****************************************************************************/ @@ -1103,8 +1049,8 @@ deny parameters before doing any parsing of the packet passed to us by the client. This prevents attacks on our parsing code from hosts not in the hosts allow list */ - if (smbd_process_limit() || - !check_access(smbd_server_fd(), lp_hostsallow(-1), lp_hostsdeny(-1))) { + if (!check_access(smbd_server_fd(), lp_hostsallow(-1), + lp_hostsdeny(-1))) { /* send a negative session response "not listening on calling name" */ static unsigned char buf[5] = {0x83, 0, 0, 1, 0x81}; DEBUG( 1, ( "Connection denied from %s\n", client_addr() ) ); Modified: branches/SAMBA_3_0/source/smbd/server.c =================================================================== --- branches/SAMBA_3_0/source/smbd/server.c 2006-08-16 09:10:54 UTC (rev 17568) +++ branches/SAMBA_3_0/source/smbd/server.c 2006-08-16 10:36:19 UTC (rev 17569) @@ -36,6 +36,7 @@ extern pstring user_socket_options; extern SIG_ATOMIC_T got_sig_term; extern SIG_ATOMIC_T reload_after_sighup; +static SIG_ATOMIC_T got_sig_cld; #ifdef WITH_DFS extern int dcelogin_atmost_once; @@ -93,6 +94,15 @@ } /**************************************************************************** + Catch a sigcld +****************************************************************************/ +static void sig_cld(int sig) +{ + got_sig_cld = 1; + sys_select_signal(SIGCLD); +} + +/**************************************************************************** Send a SIGTERM to our process group. *****************************************************************************/ @@ -189,7 +199,55 @@ } #endif /* DEVELOPER */ +struct child_pid { + struct child_pid *prev, *next; + pid_t pid; +}; +static struct child_pid *children; +static int num_children; + +static void add_child_pid(pid_t pid) +{ + struct child_pid *child; + + if (lp_max_smbd_processes() == 0) { + /* Don't bother with the child list if we don't care anyway */ + return; + } + + child = SMB_MALLOC_P(struct child_pid); + if (child == NULL) { + DEBUG(0, ("Could not add child struct -- malloc failed\n")); + return; + } + child->pid = pid; + DLIST_ADD(children, child); + num_children += 1; +} + +static void remove_child_pid(pid_t pid) +{ + struct child_pid *child; + + if (lp_max_smbd_processes() == 0) { + /* Don't bother with the child list if we don't care anyway */ + return; + } + + for (child = children; child != NULL; child = child->next) { + if (child->pid == pid) { + struct child_pid *tmp = child; + DLIST_REMOVE(children, child); + SAFE_FREE(tmp); + num_children -= 1; + return; + } + } + + DEBUG(0, ("Could not find child %d -- ignoring\n", (int)pid)); +} + /**************************************************************************** Have we reached the process limit ? ****************************************************************************/ @@ -201,27 +259,7 @@ if (!max_processes) return True; - { - TDB_CONTEXT *tdb = conn_tdb_ctx(); - int32 val; - if (!tdb) { - DEBUG(0,("allowable_number_of_smbd_processes: can't open connection tdb.\n" )); - return False; - } - - val = tdb_fetch_int32(tdb, "INFO/total_smbds"); - if (val == -1 && (tdb_error(tdb) != TDB_ERR_NOEXIST)) { - DEBUG(0,("allowable_number_of_smbd_processes: can't fetch INFO/total_smbds. Error %s\n", - tdb_errorstr(tdb) )); - return False; - } - if (val > max_processes) { - DEBUG(0,("allowable_number_of_smbd_processes: number of processes (%d) is over allowed limit (%d)\n", - val, max_processes )); - return False; - } - } - return True; + return num_children < max_processes; } /**************************************************************************** @@ -255,7 +293,7 @@ #endif /* Stop zombies */ - CatchChild(); + CatchSignal(SIGCLD, sig_cld); FD_ZERO(&listen_set); @@ -392,6 +430,15 @@ /* Ensure we respond to PING and DEBUG messages from the main smbd. */ message_dispatch(); + if (got_sig_cld) { + pid_t pid; + got_sig_cld = False; + + while ((pid = sys_waitpid(-1, NULL, WNOHANG)) > 0) { + remove_child_pid(pid); + } + } + memcpy((char *)&lfds, (char *)&listen_set, sizeof(listen_set)); @@ -421,6 +468,7 @@ for( ; num > 0; num--) { struct sockaddr addr; socklen_t in_addrlen = sizeof(addr); + pid_t child = 0; s = -1; for(i = 0; i < num_sockets; i++) { @@ -451,7 +499,8 @@ return True; if (allowable_number_of_smbd_processes() && - smbd_server_fd() != -1 && sys_fork()==0) { + smbd_server_fd() != -1 && + ((child = sys_fork())==0)) { /* Child code ... */ /* close the listening socket(s) */ @@ -499,6 +548,10 @@ smbd_set_server_fd(-1); + if (child != 0) { + add_child_pid(child); + } + /* Force parent to check log size after * spawning child. Fix from * [EMAIL PROTECTED] The @@ -640,7 +693,6 @@ yield_connection(NULL,""); respond_to_all_remaining_local_messages(); - decrement_smbd_process_count(); #ifdef WITH_DFS if (dcelogin_atmost_once) {
