Diego d'Ambra wrote: > > Attached is proposed reworked version of daemon qpsmtpd-prefork. > > Due to the number of changes I've included 2 files - one is the full > perl file (if somebody finds that more easy to use) the other is diff > against svn revision 936.
I've split your change into smaller pieces, as per your changelog, and committed them with the following modifications: > * Fixed taint issue with argument [--interface]. Since the opinions I've seen suggest that trusting the config files (and command line parameters) is OK, I've opted to just untaint the value and not validate it with Data::Validate::IP. The value will be validated when attempting the bind. Change #940. > * Fixed issue with slow or missing adjustment of children pool size on > very busy system. For this I changed the main loop to not sleep when, after some children have exited, it should check and adjust the pool size. Change #939. I don't know if it is enough. If not, please give more details about the comparison you make in your patch, where you recheck the pool if parent slept less than two seconds. > * Fixed shutdown of parent (and children) on very busy system. Sending the exit signal to the process group has been applied as change #942. > * Fixed detach option so e.g. init.d script would notice failed > startup Applied as change #938. > * Updated parent to spot and reset locked shared memory. Modified patch applied as change #941. Please review and test it. > * Clean-up of exit codes. Applied as change #942. In child however, I left SIGTERM handler set to the default handler. I think it is better this way (exit immediately, with non-zero exit code). > There are still issues that could be done better and others that are > potential fatal e.g: > * parent may not be able to adjust children pool size, if a lot of > children are exiting very rapidly after each other. > * parent tries to correct a locked shared memory condition - it would > be better to figure out, why it happens and correct that. > * during startup, the daemon should check for moribund shared memory - > currently that job must be done manually by the administrator. > * to shutdown, the parent first closes socket and then signals process > group - I'm still not convinced it does it on a very busy system. I'm wondering if, to improve robustness and decrease the code size, it would be a good idea to use Net::Server for prefork, and maybe for forkserver too. The functionality of the shared memory used by prefork could probably be replaced by the communication channel between parent and children offered by Net::Server::Prefork. Net::Server would also handle the things that are somewhat duplicated between prefork, forkserver and async: daemonize, change user, write pid file, manage sockets, connections and children.
