Re: [PATCH] mod_proxy: fix build without APR threads
On Tue, Jan 22, 2019 at 10:49:27AM -0600, William A Rowe Jr wrote: > On Tue, Jan 22, 2019 at 10:30 AM Stefan Sperling wrote: > > > On Tue, Jan 08, 2019 at 03:46:48PM +0100, Stefan Sperling wrote: > > > mod_proxy fails to compile when APR doesn't have thread support. > > > I don't know if this is supposed to be a supported configuration, > > > but this problem did not exist with HTTPD 2.2; it showed up in 2.4. > > > > How early in 2.4? I believe no-threads/prefork/mod_proxy had been > a supported combo during 2.4.1+ lifetime. It should be corrected. I don't know when it started. I had been lazy and kept compiling my SVN dev builds with httpd 2.2 for ages. I only switched that setup to the 2.4.x line with 2.4.37, because I finally ran into enough trouble keeping my 2.2-based setup in working condition. > mod_proxy_balancer doesn't make any sense, of course, so if it > has similar issues, those aren't a worry. I patched mod_proxy_balancer since it had compilation failures in my no-threads configuration. Are you saying mod_proxy_balancer should not be compiled in the first place if threads are disabled in APR? > > The patch below adds checks for APR_HAS_THREADS and passes test > > > builds with both threaded and non-threaded APR from APR's trunk. > > > Is this the right approach? > > > > > > I also found that the http2 module won't build without thread support. > > > > This is rather known. Early in the mod_http2 adoption, it was decided > that it would not support mod_prefork. Since it only supports threaded > MPM's, that's fine, it can be disabled for no-threads. OK, that's fine as it is then. > > > I can simply omit http2 from my SVN dev builds, for now. But mod_proxy > > > is required by mod_dav_svn for SVN's write-through proxy feature. > > > > > > Any feedback on this? Should I just commit it if I don't hear anything? > > > > You can commit what you believe is the right fix to trunk, and propose > this for backport in STATUS for 2.4 branch for others to consider, test > and collect the 3 +1's needed for backporting. Will do. Thanks!
Re: [PATCH] mod_proxy: fix build without APR threads
On Tue, Jan 22, 2019 at 10:30 AM Stefan Sperling wrote: > On Tue, Jan 08, 2019 at 03:46:48PM +0100, Stefan Sperling wrote: > > mod_proxy fails to compile when APR doesn't have thread support. > > I don't know if this is supposed to be a supported configuration, > > but this problem did not exist with HTTPD 2.2; it showed up in 2.4. > How early in 2.4? I believe no-threads/prefork/mod_proxy had been a supported combo during 2.4.1+ lifetime. It should be corrected. mod_proxy_balancer doesn't make any sense, of course, so if it has similar issues, those aren't a worry. > The patch below adds checks for APR_HAS_THREADS and passes test > > builds with both threaded and non-threaded APR from APR's trunk. > > Is this the right approach? > > > > I also found that the http2 module won't build without thread support. > This is rather known. Early in the mod_http2 adoption, it was decided that it would not support mod_prefork. Since it only supports threaded MPM's, that's fine, it can be disabled for no-threads. > > I can simply omit http2 from my SVN dev builds, for now. But mod_proxy > > is required by mod_dav_svn for SVN's write-through proxy feature. > > Any feedback on this? Should I just commit it if I don't hear anything? > You can commit what you believe is the right fix to trunk, and propose this for backport in STATUS for 2.4 branch for others to consider, test and collect the 3 +1's needed for backporting.
Re: [PATCH] mod_proxy: fix build without APR threads
On Tue, Jan 08, 2019 at 03:46:48PM +0100, Stefan Sperling wrote: > mod_proxy fails to compile when APR doesn't have thread support. > I don't know if this is supposed to be a supported configuration, > but this problem did not exist with HTTPD 2.2; it showed up in 2.4. > > The patch below adds checks for APR_HAS_THREADS and passes test > builds with both threaded and non-threaded APR from APR's trunk. > Is this the right approach? > > I also found that the http2 module won't build without thread support. > I can simply omit http2 from my SVN dev builds, for now. But mod_proxy > is required by mod_dav_svn for SVN's write-through proxy feature. Any feedback on this? Should I just commit it if I don't hear anything? > Index: modules/proxy/mod_proxy.h > === > --- modules/proxy/mod_proxy.h (revision 1850749) > +++ modules/proxy/mod_proxy.h (working copy) > @@ -472,7 +472,9 @@ struct proxy_worker { > proxy_conn_pool *cp;/* Connection pool to use */ > proxy_worker_shared *s; /* Shared data */ > proxy_balancer *balancer; /* which balancer am I in? */ > +#if APR_HAS_THREADS > apr_thread_mutex_t *tmutex; /* Thread lock for updating address cache */ > +#endif > void*context; /* general purpose storage */ > ap_conf_vector_t *section_config; /* -section wherein defined */ > }; > @@ -528,8 +530,10 @@ struct proxy_balancer { > proxy_hashes hash; > apr_time_t wupdated;/* timestamp of last change to workers list > */ > proxy_balancer_method *lbmethod; > +#if APR_HAS_THREADS > apr_global_mutex_t *gmutex; /* global lock for updating list of workers > */ > apr_thread_mutex_t *tmutex; /* Thread lock for updating shm */ > +#endif > proxy_server_conf *sconf; > void*context;/* general purpose storage */ > proxy_balancer_shared *s;/* Shared data */ > Index: modules/proxy/mod_proxy_balancer.c > === > --- modules/proxy/mod_proxy_balancer.c(revision 1850749) > +++ modules/proxy/mod_proxy_balancer.c(working copy) > @@ -346,6 +346,7 @@ static proxy_worker *find_best_worker(proxy_balanc > proxy_worker *candidate = NULL; > apr_status_t rv; > > +#if APR_HAS_THREADS > if ((rv = PROXY_THREAD_LOCK(balancer)) != APR_SUCCESS) { > ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01163) >"%s: Lock failed for find_best_worker()", > @@ -352,6 +353,7 @@ static proxy_worker *find_best_worker(proxy_balanc >balancer->s->name); > return NULL; > } > +#endif > > candidate = (*balancer->lbmethod->finder)(balancer, r); > > @@ -358,11 +360,13 @@ static proxy_worker *find_best_worker(proxy_balanc > if (candidate) > candidate->s->elected++; > > +#if APR_HAS_THREADS > if ((rv = PROXY_THREAD_UNLOCK(balancer)) != APR_SUCCESS) { > ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01164) >"%s: Unlock failed for find_best_worker()", >balancer->s->name); > } > +#endif > > if (candidate == NULL) { > /* All the workers are in error state or disabled. > @@ -492,11 +496,13 @@ static int proxy_balancer_pre_request(proxy_worker > /* Step 2: Lock the LoadBalancer > * XXX: perhaps we need the process lock here > */ > +#if APR_HAS_THREADS > if ((rv = PROXY_THREAD_LOCK(*balancer)) != APR_SUCCESS) { > ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01166) >"%s: Lock failed for pre_request", > (*balancer)->s->name); > return DECLINED; > } > +#endif > > /* Step 3: force recovery */ > force_recovery(*balancer, r->server); > @@ -557,20 +563,24 @@ static int proxy_balancer_pre_request(proxy_worker > ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01167) >"%s: All workers are in error state for route > (%s)", >(*balancer)->s->name, route); > +#if APR_HAS_THREADS > if ((rv = PROXY_THREAD_UNLOCK(*balancer)) != APR_SUCCESS) { > ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01168) >"%s: Unlock failed for pre_request", >(*balancer)->s->name); > } > +#endif > return HTTP_SERVICE_UNAVAILABLE; > } > } > > +#if APR_HAS_THREADS > if ((rv = PROXY_THREAD_UNLOCK(*balancer)) != APR_SUCCESS) { > ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01169) >"%s: Unlock failed for pre_request", >(*balancer)->s->name); > } > +#endif > if (!*worker) { > runtime = find_best_worker(*balancer, r); > if (!runtime) { > @@ -644,6 +654,7 @@ static int
[PATCH] mod_proxy: fix build without APR threads
mod_proxy fails to compile when APR doesn't have thread support. I don't know if this is supposed to be a supported configuration, but this problem did not exist with HTTPD 2.2; it showed up in 2.4. The patch below adds checks for APR_HAS_THREADS and passes test builds with both threaded and non-threaded APR from APR's trunk. Is this the right approach? I also found that the http2 module won't build without thread support. I can simply omit http2 from my SVN dev builds, for now. But mod_proxy is required by mod_dav_svn for SVN's write-through proxy feature. Index: modules/proxy/mod_proxy.h === --- modules/proxy/mod_proxy.h (revision 1850749) +++ modules/proxy/mod_proxy.h (working copy) @@ -472,7 +472,9 @@ struct proxy_worker { proxy_conn_pool *cp;/* Connection pool to use */ proxy_worker_shared *s; /* Shared data */ proxy_balancer *balancer; /* which balancer am I in? */ +#if APR_HAS_THREADS apr_thread_mutex_t *tmutex; /* Thread lock for updating address cache */ +#endif void*context; /* general purpose storage */ ap_conf_vector_t *section_config; /* -section wherein defined */ }; @@ -528,8 +530,10 @@ struct proxy_balancer { proxy_hashes hash; apr_time_t wupdated;/* timestamp of last change to workers list */ proxy_balancer_method *lbmethod; +#if APR_HAS_THREADS apr_global_mutex_t *gmutex; /* global lock for updating list of workers */ apr_thread_mutex_t *tmutex; /* Thread lock for updating shm */ +#endif proxy_server_conf *sconf; void*context;/* general purpose storage */ proxy_balancer_shared *s;/* Shared data */ Index: modules/proxy/mod_proxy_balancer.c === --- modules/proxy/mod_proxy_balancer.c (revision 1850749) +++ modules/proxy/mod_proxy_balancer.c (working copy) @@ -346,6 +346,7 @@ static proxy_worker *find_best_worker(proxy_balanc proxy_worker *candidate = NULL; apr_status_t rv; +#if APR_HAS_THREADS if ((rv = PROXY_THREAD_LOCK(balancer)) != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01163) "%s: Lock failed for find_best_worker()", @@ -352,6 +353,7 @@ static proxy_worker *find_best_worker(proxy_balanc balancer->s->name); return NULL; } +#endif candidate = (*balancer->lbmethod->finder)(balancer, r); @@ -358,11 +360,13 @@ static proxy_worker *find_best_worker(proxy_balanc if (candidate) candidate->s->elected++; +#if APR_HAS_THREADS if ((rv = PROXY_THREAD_UNLOCK(balancer)) != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01164) "%s: Unlock failed for find_best_worker()", balancer->s->name); } +#endif if (candidate == NULL) { /* All the workers are in error state or disabled. @@ -492,11 +496,13 @@ static int proxy_balancer_pre_request(proxy_worker /* Step 2: Lock the LoadBalancer * XXX: perhaps we need the process lock here */ +#if APR_HAS_THREADS if ((rv = PROXY_THREAD_LOCK(*balancer)) != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01166) "%s: Lock failed for pre_request", (*balancer)->s->name); return DECLINED; } +#endif /* Step 3: force recovery */ force_recovery(*balancer, r->server); @@ -557,20 +563,24 @@ static int proxy_balancer_pre_request(proxy_worker ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(01167) "%s: All workers are in error state for route (%s)", (*balancer)->s->name, route); +#if APR_HAS_THREADS if ((rv = PROXY_THREAD_UNLOCK(*balancer)) != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01168) "%s: Unlock failed for pre_request", (*balancer)->s->name); } +#endif return HTTP_SERVICE_UNAVAILABLE; } } +#if APR_HAS_THREADS if ((rv = PROXY_THREAD_UNLOCK(*balancer)) != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01169) "%s: Unlock failed for pre_request", (*balancer)->s->name); } +#endif if (!*worker) { runtime = find_best_worker(*balancer, r); if (!runtime) { @@ -644,6 +654,7 @@ static int proxy_balancer_post_request(proxy_worke apr_status_t rv; +#if APR_HAS_THREADS if ((rv = PROXY_THREAD_LOCK(balancer)) != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_ERR, rv, r, APLOGNO(01173) "%s: Lock failed for post_request", @@ -650,6 +661,7 @@ static int proxy_balancer_post_request(proxy_worke