Jim Jagielski <[EMAIL PROTECTED]> writes:
> Comments are welcome... I'd like to commit the patch this week. At
> that point, we port to 2.0, which already has a lot of the
> work done (just need to do the default stuff and handle SingleListen)
which "default stuff" is needed in 2.0?
why is SingleListen needed?
> Index: src/include/ap_config.h
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/include/ap_config.h,v
> retrieving revision 1.309
> diff -u -r1.309 ap_config.h
> --- src/include/ap_config.h 2001/06/22 12:43:53 1.309
> +++ src/include/ap_config.h 2001/08/20 18:19:42
> @@ -179,10 +179,10 @@
> #define NO_KILLPG
> #undef NO_SETSID
> #define bzero(a,b) memset(a,0,b)
> -#if !defined(USE_SYSVSEM_SERIALIZED_ACCEPT) && \
> - !defined(USE_PTHREAD_SERIALIZED_ACCEPT)
> +#define USE_SYSVSEM_SERIALIZED_ACCEPT
> +#define USE_PTHREAD_SERIALIZED_ACCEPT
Doesn't HAVE_foo_SERIALIZED_ACCEPT make more sense than
USE_foo_SERIALIZED_ACCEPT? I guess changing the meaning of
USE_foo_SERIALIZED_ACCEPT helps minimize the changes.
> #define USE_FCNTL_SERIALIZED_ACCEPT
> -#endif
> +#define DEFAULT_SERIALIZED_ACCEPT_METHOD fcntl
I guess we only have to set DEFAULT_SERIALIZED_ACCEPT_METHOD on
systems where there is more than one choice?
> #define NEED_UNION_SEMUN
> #define HAVE_MMAP 1
> #define USE_MMAP_SCOREBOARD
> @@ -433,6 +433,9 @@
> #define JMP_BUF jmp_buf
> #define USE_LONGJMP
> #define USE_FLOCK_SERIALIZED_ACCEPT
> +#define USE_FCNTL_SERIALIZED_ACCEPT
> +#define USE_NONE_SERIALIZED_ACCEPT
> +#define DEFAULT_SERIALIZED_ACCEPT_METHOD flock
> #define SINGLE_LISTEN_UNSERIALIZED_ACCEPT
So what decides when "USE_NONE_SERIALIZED_ACCEPT" is a choice? I'd
think that if you allow it anywhere you'd allow it everywhere, and
that there'd be no need to define anything in ap_config.h.
> #elif defined(LINUX)
I guess we need more defines here, and on various platforms. What is
the philosophy? We could do some test compiles or just leave it like
this, and anybody that gives a s%^& can add #define USE_foo for their
favorite platforms to make foo available.
> Index: src/include/http_main.h
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/include/http_main.h,v
> retrieving revision 1.36
> diff -u -r1.36 http_main.h
> --- src/include/http_main.h 2001/01/15 17:04:34 1.36
> +++ src/include/http_main.h 2001/08/20 18:19:42
> @@ -130,6 +130,11 @@
>
> void setup_signal_names(char *prefix);
>
> +/* functions for determination and setting of accept() mutexing */
> +char *default_mutex_method(void);
> +char *init_mutex_method(char *t);
> +char *init_single_listen(int flag);
prefix with "ap_"... yeah, not everything in 1.3 is
namespace-protected, but it is trivial for new functions.
> Index: src/main/http_core.c
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/main/http_core.c,v
> retrieving revision 1.296
> diff -u -r1.296 http_core.c
> --- src/main/http_core.c 2001/07/13 07:32:44 1.296
> +++ src/main/http_core.c 2001/08/20 18:19:51
> @@ -1135,6 +1135,14 @@
> }
> return NULL;
> }
> +static const char *set_accept_mutex(cmd_parms *cmd, void *dummy, char *arg)
> +{
> + return init_mutex_method(arg);
> +}
> +static const char *set_single_listen(cmd_parms *cmd, void *dummy, int flag)
> +{
> + return init_single_listen(flag ? 1 : 0);
> +}
>
> static const char *set_document_root(cmd_parms *cmd, void *dummy, char *arg)
> {
> @@ -3215,6 +3223,37 @@
> (void*)XtOffsetOf(core_dir_config, limit_req_body),
> OR_ALL, TAKE1,
> "Limit (in bytes) on maximum size of request message body" },
> +{ "AcceptMutex", set_accept_mutex, NULL, RSRC_CONF, TAKE1,
> + "Serialized Accept Mutex; the methods "
> +#ifdef USE_FCNTL_SERIALIZED_ACCEPT
> + "'fcntl' "
> +#endif
> +#ifdef USE_FLOCK_SERIALIZED_ACCEPT
> + "'flock' "
> +#endif
> +#ifdef USE_USLOCK_SERIALIZED_ACCEPT
> + "'uslock "
> +#endif
> +#ifdef USE_SYSVSEM_SERIALIZED_ACCEPT
> + "'sysvsem' "
> +#endif
> +#ifdef USE_PTHREAD_SERIALIZED_ACCEPT
> + "'pthread' "
> +#endif
> +#ifdef USE_OS2SEM_SERIALIZED_ACCEPT
> + "'os2sem' "
> +#endif
> +#ifdef USE_TPF_CORE_SERIALIZED_ACCEPT
> + "'tpfcore' "
> +#endif
> +#ifdef USE_NONE_SERIALIZED_ACCEPT
> + "'none' "
> +#endif
> + "are compiled in"
> +},
> +{ "SingleListen",set_single_listen,NULL,RSRC_CONF,FLAG,
> + "On - to serialize accept and Off to unserialize accept"
> +},
>
> /* EBCDIC Conversion directives: */
> #ifdef CHARSET_EBCDIC
> Index: src/main/http_main.c
> ===================================================================
> RCS file: /home/cvs/apache-1.3/src/main/http_main.c,v
> retrieving revision 1.546
> diff -u -r1.546 http_main.c
> --- src/main/http_main.c 2001/08/10 01:14:29 1.546
> +++ src/main/http_main.c 2001/08/20 18:20:05
> @@ -258,6 +258,17 @@
> time_t ap_restart_time=0;
> API_VAR_EXPORT int ap_suexec_enabled = 0;
> int ap_listenbacklog=0;
> +/* On some architectures it's safe to do unserialized accept()s in the single
> + * Listen case. But it's never safe to do it in the case where there's
> + * multiple Listen statements. Define SINGLE_LISTEN_UNSERIALIZED_ACCEPT
> + * when it's safe in the single Listen case.
> + */
> +#ifdef SINGLE_LISTEN_UNSERIALIZED_ACCEPT
> +int ap_single_listen = 1;
> +#else
> +int ap_single_listen = 0;
> +#endif
> +
> #ifdef SO_ACCEPTFILTER
> int ap_acceptfilter =
> #ifdef AP_ACCEPTFILTER_OFF
> @@ -513,14 +524,12 @@
> #endif
>
> #if defined (USE_USLOCK_SERIALIZED_ACCEPT)
> -
> #include <ulocks.h>
> -
> static ulock_t uslock = NULL;
>
> -#define accept_mutex_child_init(x)
> +#define accept_mutex_child_init_uslock(x)
>
> -static void accept_mutex_init(pool *p)
> +static void accept_mutex_init_uslock(pool *p)
> {
> ptrdiff_t old;
> usptr_t *us;
> @@ -551,7 +560,7 @@
> }
> }
>
> -static void accept_mutex_on(void)
> +static void accept_mutex_on_uslock(void)
> {
> switch (ussetlock(uslock)) {
> case 1:
> @@ -566,15 +575,16 @@
> }
> }
>
> -static void accept_mutex_off(void)
> +static void accept_mutex_off_uslock(void)
> {
> if (usunsetlock(uslock) == -1) {
> perror("usunsetlock");
> clean_child_exit(APEXIT_CHILDFATAL);
> }
> }
> +#endif
>
> -#elif defined (USE_PTHREAD_SERIALIZED_ACCEPT)
> +#if defined (USE_PTHREAD_SERIALIZED_ACCEPT)
>
> /* This code probably only works on Solaris ... but it works really fast
> * on Solaris. Note that pthread mutexes are *NOT* released when a task
> @@ -589,7 +599,7 @@
> static sigset_t accept_block_mask;
> static sigset_t accept_previous_mask;
>
> -static void accept_mutex_child_cleanup(void *foo)
> +static void accept_mutex_child_cleanup_pthread(void *foo)
> {
> if (accept_mutex != (void *)(caddr_t)-1
> && have_accept_mutex) {
> @@ -597,12 +607,12 @@
> }
> }
>
> -static void accept_mutex_child_init(pool *p)
> +static void accept_mutex_child_init_pthread(pool *p)
> {
> - ap_register_cleanup(p, NULL, accept_mutex_child_cleanup, ap_null_cleanup);
> + ap_register_cleanup(p, NULL, accept_mutex_child_cleanup_pthread,
>ap_null_cleanup);
> }
>
> -static void accept_mutex_cleanup(void *foo)
> +static void accept_mutex_cleanup_pthread(void *foo)
> {
> if (accept_mutex != (void *)(caddr_t)-1
> && munmap((caddr_t) accept_mutex, sizeof(*accept_mutex))) {
> @@ -611,7 +621,7 @@
> accept_mutex = (void *)(caddr_t)-1;
> }
>
> -static void accept_mutex_init(pool *p)
> +static void accept_mutex_init_pthread(pool *p)
> {
> pthread_mutexattr_t mattr;
> int fd;
> @@ -645,10 +655,10 @@
> sigdelset(&accept_block_mask, SIGHUP);
> sigdelset(&accept_block_mask, SIGTERM);
> sigdelset(&accept_block_mask, SIGUSR1);
> - ap_register_cleanup(p, NULL, accept_mutex_cleanup, ap_null_cleanup);
> + ap_register_cleanup(p, NULL, accept_mutex_cleanup_pthread, ap_null_cleanup);
> }
>
> -static void accept_mutex_on(void)
> +static void accept_mutex_on_pthread(void)
> {
> int err;
>
> @@ -670,7 +680,7 @@
> ap_unblock_alarms();
> }
>
> -static void accept_mutex_off(void)
> +static void accept_mutex_off_pthread(void)
> {
> int err;
>
> @@ -691,8 +701,9 @@
> clean_child_exit(1);
> }
> }
> +#endif
>
> -#elif defined (USE_SYSVSEM_SERIALIZED_ACCEPT)
> +#if defined (USE_SYSVSEM_SERIALIZED_ACCEPT)
>
> #include <sys/types.h>
> #include <sys/ipc.h>
> @@ -716,7 +727,7 @@
> * means we have to be sure to clean this up or else we'll leak
> * semaphores.
> */
> -static void accept_mutex_cleanup(void *foo)
> +static void accept_mutex_cleanup_sysvsem(void *foo)
> {
> union semun ick;
>
> @@ -727,9 +738,9 @@
> semctl(sem_id, 0, IPC_RMID, ick);
> }
>
> -#define accept_mutex_child_init(x)
> +#define accept_mutex_child_init_sysvsem(x)
>
> -static void accept_mutex_init(pool *p)
> +static void accept_mutex_init_sysvsem(pool *p)
> {
> union semun ick;
> struct semid_ds buf;
> @@ -758,7 +769,7 @@
> exit(APEXIT_INIT);
> }
> }
> - ap_register_cleanup(p, NULL, accept_mutex_cleanup, ap_null_cleanup);
> + ap_register_cleanup(p, NULL, accept_mutex_cleanup_sysvsem, ap_null_cleanup);
>
> /* pre-initialize these */
> op_on.sem_num = 0;
> @@ -769,7 +780,7 @@
> op_off.sem_flg = SEM_UNDO;
> }
>
> -static void accept_mutex_on(void)
> +static void accept_mutex_on_sysvsem(void)
> {
> while (semop(sem_id, &op_on, 1) < 0) {
> if (errno != EINTR) {
> @@ -779,7 +790,7 @@
> }
> }
>
> -static void accept_mutex_off(void)
> +static void accept_mutex_off_sysvsem(void)
> {
> while (semop(sem_id, &op_off, 1) < 0) {
> if (errno != EINTR) {
> @@ -788,20 +799,21 @@
> }
> }
> }
> +#endif
>
> -#elif defined(USE_FCNTL_SERIALIZED_ACCEPT)
> +#if defined(USE_FCNTL_SERIALIZED_ACCEPT)
> static struct flock lock_it;
> static struct flock unlock_it;
>
> static int lock_fd = -1;
>
> -#define accept_mutex_child_init(x)
> +#define accept_mutex_child_init_fcntl(x)
>
> /*
> * Initialize mutex lock.
> * Must be safe to call this on a restart.
> */
> -static void accept_mutex_init(pool *p)
> +static void accept_mutex_init_fcntl(pool *p)
> {
>
> lock_it.l_whence = SEEK_SET; /* from current point */
> @@ -825,7 +837,7 @@
> unlink(ap_lock_fname);
> }
>
> -static void accept_mutex_on(void)
> +static void accept_mutex_on_fcntl(void)
> {
> int ret;
>
> @@ -842,7 +854,7 @@
> }
> }
>
> -static void accept_mutex_off(void)
> +static void accept_mutex_off_fcntl(void)
> {
> int ret;
>
> @@ -857,12 +869,13 @@
> clean_child_exit(APEXIT_CHILDFATAL);
> }
> }
> +#endif
>
> -#elif defined(USE_FLOCK_SERIALIZED_ACCEPT)
> +#if defined(USE_FLOCK_SERIALIZED_ACCEPT)
>
> -static int lock_fd = -1;
> +static int flock_fd = -1;
>
> -static void accept_mutex_cleanup(void *foo)
> +static void accept_mutex_cleanup_flock(void *foo)
> {
> unlink(ap_lock_fname);
> }
> @@ -871,11 +884,11 @@
> * Initialize mutex lock.
> * Done by each child at it's birth
> */
> -static void accept_mutex_child_init(pool *p)
> +static void accept_mutex_child_init_flock(pool *p)
> {
>
> - lock_fd = ap_popenf(p, ap_lock_fname, O_WRONLY, 0600);
> - if (lock_fd == -1) {
> + flock_fd = ap_popenf(p, ap_lock_fname, O_WRONLY, 0600);
> + if (flock_fd == -1) {
> ap_log_error(APLOG_MARK, APLOG_EMERG, server_conf,
> "Child cannot open lock file: %s", ap_lock_fname);
> clean_child_exit(APEXIT_CHILDINIT);
> @@ -886,24 +899,24 @@
> * Initialize mutex lock.
> * Must be safe to call this on a restart.
> */
> -static void accept_mutex_init(pool *p)
> +static void accept_mutex_init_flock(pool *p)
> {
> expand_lock_fname(p);
> unlink(ap_lock_fname);
> - lock_fd = ap_popenf(p, ap_lock_fname, O_CREAT | O_WRONLY | O_EXCL, 0600);
> - if (lock_fd == -1) {
> + flock_fd = ap_popenf(p, ap_lock_fname, O_CREAT | O_WRONLY | O_EXCL, 0600);
> + if (flock_fd == -1) {
> ap_log_error(APLOG_MARK, APLOG_EMERG, server_conf,
> "Parent cannot open lock file: %s", ap_lock_fname);
> exit(APEXIT_INIT);
> }
> - ap_register_cleanup(p, NULL, accept_mutex_cleanup, ap_null_cleanup);
> + ap_register_cleanup(p, NULL, accept_mutex_cleanup_flock, ap_null_cleanup);
> }
>
> -static void accept_mutex_on(void)
> +static void accept_mutex_on_flock(void)
> {
> int ret;
>
> - while ((ret = flock(lock_fd, LOCK_EX)) < 0 && errno == EINTR)
> + while ((ret = flock(flock_fd, LOCK_EX)) < 0 && errno == EINTR)
> continue;
>
> if (ret < 0) {
> @@ -913,20 +926,21 @@
> }
> }
>
> -static void accept_mutex_off(void)
> +static void accept_mutex_off_flock(void)
> {
> - if (flock(lock_fd, LOCK_UN) < 0) {
> + if (flock(flock_fd, LOCK_UN) < 0) {
> ap_log_error(APLOG_MARK, APLOG_EMERG, server_conf,
> "flock: LOCK_UN: Error freeing accept lock. Exiting!");
> clean_child_exit(APEXIT_CHILDFATAL);
> }
> }
> +#endif
>
> -#elif defined(USE_OS2SEM_SERIALIZED_ACCEPT)
> +#if defined(USE_OS2SEM_SERIALIZED_ACCEPT)
>
> static HMTX lock_sem = -1;
>
> -static void accept_mutex_cleanup(void *foo)
> +static void accept_mutex_cleanup_os2sem(void *foo)
> {
> DosReleaseMutexSem(lock_sem);
> DosCloseMutexSem(lock_sem);
> @@ -936,7 +950,7 @@
> * Initialize mutex lock.
> * Done by each child at it's birth
> */
> -static void accept_mutex_child_init(pool *p)
> +static void accept_mutex_child_init_os2sem(pool *p)
> {
> int rc = DosOpenMutexSem(NULL, &lock_sem);
>
> @@ -945,7 +959,7 @@
> "Child cannot open lock semaphore, rc=%d", rc);
> clean_child_exit(APEXIT_CHILDINIT);
> } else {
> - ap_register_cleanup(p, NULL, accept_mutex_cleanup, ap_null_cleanup);
> + ap_register_cleanup(p, NULL, accept_mutex_cleanup_os2sem, ap_null_cleanup);
> }
> }
>
> @@ -953,7 +967,7 @@
> * Initialize mutex lock.
> * Must be safe to call this on a restart.
> */
> -static void accept_mutex_init(pool *p)
> +static void accept_mutex_init_os2sem(pool *p)
> {
> int rc = DosCreateMutexSem(NULL, &lock_sem, DC_SEM_SHARED, FALSE);
>
> @@ -963,10 +977,10 @@
> exit(APEXIT_INIT);
> }
>
> - ap_register_cleanup(p, NULL, accept_mutex_cleanup, ap_null_cleanup);
> + ap_register_cleanup(p, NULL, accept_mutex_cleanup_os2sem, ap_null_cleanup);
> }
>
> -static void accept_mutex_on(void)
> +static void accept_mutex_on_os2sem(void)
> {
> int rc = DosRequestMutexSem(lock_sem, SEM_INDEFINITE_WAIT);
>
> @@ -977,7 +991,7 @@
> }
> }
>
> -static void accept_mutex_off(void)
> +static void accept_mutex_off_os2sem(void)
> {
> int rc = DosReleaseMutexSem(lock_sem);
>
> @@ -987,65 +1001,204 @@
> clean_child_exit(APEXIT_CHILDFATAL);
> }
> }
> +#endif
>
> -#elif defined(USE_TPF_CORE_SERIALIZED_ACCEPT)
> +#if defined(USE_TPF_CORE_SERIALIZED_ACCEPT)
>
> static int tpf_core_held;
>
> -static void accept_mutex_cleanup(void *foo)
> +static void accept_mutex_cleanup_tpfcore(void *foo)
> {
> if(tpf_core_held)
> coruc(RESOURCE_KEY);
> }
>
> -#define accept_mutex_init(x)
> +#define accept_mutex_init_tpfcore(x)
>
> -static void accept_mutex_child_init(pool *p)
> +static void accept_mutex_child_init_tpfcore(pool *p)
> {
> - ap_register_cleanup(p, NULL, accept_mutex_cleanup, ap_null_cleanup);
> + ap_register_cleanup(p, NULL, accept_mutex_cleanup_tpfcore, ap_null_cleanup);
> tpf_core_held = 0;
> }
>
> -static void accept_mutex_on(void)
> +static void accept_mutex_on_tpfcore(void)
> {
> corhc(RESOURCE_KEY);
> tpf_core_held = 1;
> ap_check_signals();
> }
>
> -static void accept_mutex_off(void)
> +static void accept_mutex_off_tpfcore(void)
> {
> coruc(RESOURCE_KEY);
> tpf_core_held = 0;
> ap_check_signals();
> }
> +#endif
>
> -#else
> -/* Default --- no serialization. Other methods *could* go here,
> - * as #elifs...
> - */
> #if !defined(MULTITHREAD)
> /* Multithreaded systems don't complete between processes for
> * the sockets. */
> #define NO_SERIALIZED_ACCEPT
> -#define accept_mutex_child_init(x)
> -#define accept_mutex_init(x)
> -#define accept_mutex_on()
> -#define accept_mutex_off()
> +#define USE_NONE_CORE_SERIALIZED_ACCEPT
> +/* The below are actually not-needed, but are here for completeness.
> + * We take care of this in USE_NONE_CORE_SERIALIZED_ACCEPT */
> +#define accept_mutex_child_init_none(x)
> +#define accept_mutex_init_none(x)
> +#define accept_mutex_on_none()
> +#define accept_mutex_off_none()
> +#endif
> +
> +void (*accept_mutex_child_init_fptr)(pool *);
> +void (*accept_mutex_init_fptr)(pool *);
> +void (*accept_mutex_off_fptr)(void);
> +void (*accept_mutex_on_fptr)(void);
> +
> +#define AP_FPTR1(x,y) { if (x) ((* x)(y)); }
> +#define AP_FPTR0(x) { if (x) ((* x)()); }
> +
> +#define accept_mutex_child_init(x) AP_FPTR1(accept_mutex_child_init_fptr,x)
> +#define accept_mutex_init(x) AP_FPTR1(accept_mutex_init_fptr,x)
> +#define accept_mutex_off() AP_FPTR0(accept_mutex_off_fptr)
> +#define accept_mutex_on() AP_FPTR0(accept_mutex_on_fptr)
> +
> +char *default_mutex_method(void)
> +{
> + char *t;
> +#if defined DEFAULT_SERIALIZED_ACCEPT_METHOD
> + t = "DEFAULT_SERIALIZED_ACCEPT_METHOD";
Don't you want something like #DEFAULT_SERIALIZED_ACCEPT_METHOD (or
maybe with "##" instead of "#" to convert the value into a string?
> +#else
> + t = "default";
> #endif
> +#if defined USE_USLOCK_SERIALIZED_ACCEPT
> + if ((!(strcasecmp(t,"default"))) || (!(strcasecmp(t,"uslock")))) {
> + return "uslock";
> + } else
I'm confused. How do we know that if t is "default" we should use
uslock? Is it the order of the checks? I guess it is a code error to
define more than one USE_foo_SERIALIZED_ACCEPT for the platform but
not define which one is the default? Should we catch that here rather
than selecting the first match?
> + return "Request serialized accept method not available";
"Requested", not "Request"
> + return "Request serialized accept method not available";
same as previous comment, but I'd think one of these should never
fail... how about ap_assert() in one of them instead of returning an
error that we shouldn't have logic to check for?
--
Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
