Re: svn commit: r741947 - in /httpd/httpd/trunk/modules/mappers: config9.m4 mod_watchdog.c mod_watchdog.h

2009-02-08 Thread Ruediger Pluem


On 02/07/2009 08:51 PM, mt...@apache.org wrote:
 Author: mturk
 Date: Sat Feb  7 19:51:52 2009
 New Revision: 741947
 
 URL: http://svn.apache.org/viewvc?rev=741947view=rev
 Log:
 Add watchdog module
 
 Added:
 httpd/httpd/trunk/modules/mappers/mod_watchdog.c   (with props)
 httpd/httpd/trunk/modules/mappers/mod_watchdog.h   (with props)
 Modified:
 httpd/httpd/trunk/modules/mappers/config9.m4
 

 Added: httpd/httpd/trunk/modules/mappers/mod_watchdog.c
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/mappers/mod_watchdog.c?rev=741947view=auto
 ==

 +/*--*/
 +/*  
 */
 +/* Main watchdog worker thread. 
 */
 +/* For singleton workers child thread theat first obtains the process   
 */
 +/* mutex is running. Threads in other child's are locked on mutex.  
 */
 +/*  
 */
 +/*--*/
 +static void* APR_THREAD_FUNC wd_worker(apr_thread_t *thread, void *data)
 +{
 +ap_watchdog_t *w = (ap_watchdog_t *)data;
 +apr_status_t rv;
 +int locked = 0;
 +int probed = 0;
 +int inited = 0;
 +
 +w-pool = apr_thread_pool_get(thread);
 +w-is_running = 1;
 +
 +apr_thread_mutex_unlock(w-startup);
 +if (w-mutex) {
 +while (w-is_running) {
 +rv = apr_proc_mutex_trylock(w-mutex);
 +if (rv == APR_SUCCESS) {
 +if (probed) {
 +/* Sleep after we were locked
 + * up to 1 second. Httpd can be
 + * in the middle of shutdown, and
 + * our child didn't yet received
 + * the shutdown signal.
 + */
 +probed = 10;
 +while (w-is_running  probed  0) {
 +apr_sleep(AP_WD_TM_INTERVAL);
 +probed--;
 +}
 +}
 +locked = 1;
 +break;
 +}
 +probed = 1;
 +apr_sleep(AP_WD_TM_SLICE);
 +}
 +}
 +if (w-is_running) {
 +watchdog_list_t *wl = w-callbacks;
 +ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, wd_server_conf-s,
 + %sWatchdog (%s) running (%d),
 + w-singleton ? Singleton : ,
 + w-name, getpid());
 +apr_time_clock_hires(w-pool);
 +if (wl) {
 +apr_pool_t *ctx = NULL;
 +apr_pool_create(ctx, w-pool);
 +while (wl  w-is_running) {
 +/* Execute watchdog callback */
 +wl-status = (*wl-callback_fn)(AP_WATCHDOG_STATE_STARTING,
 +(void *)wl-data, ctx);
 +wl = wl-next;
 +}
 +apr_pool_destroy(ctx);
 +}
 +else {
 +ap_run_watchdog_init(wd_server_conf-s, w-name, w-pool);
 +inited = 1;
 +}
 +}
 +
 +/* Main execution loop */
 +while (w-is_running) {
 +apr_pool_t *ctx = NULL;
 +apr_time_t curr;
 +watchdog_list_t *wl = w-callbacks;
 +
 +apr_sleep(AP_WD_TM_SLICE);
 +if (!w-is_running) {
 +break;
 +}
 +curr = apr_time_now() - AP_WD_TM_SLICE;
 +while (wl  w-is_running) {
 +if (wl-status == APR_SUCCESS) {
 +wl-step += (apr_time_now() - curr);
 +if (wl-step = wl-interval) {
 +if (!ctx)

Is this check really needed? ctx should be always NULL here.

 +apr_pool_create(ctx, w-pool);
 +wl-step = 0;
 +/* Execute watchdog callback */
 +wl-status = 
 (*wl-callback_fn)(AP_WATCHDOG_STATE_RUNNING,
 +(void *)wl-data, ctx);
 +}
 +}
 +wl = wl-next;
 +}
 +if (w-is_running  w-callbacks == NULL) {
 +/* This is hook mode watchdog
 + * running on WatchogInterval
 + */
 +w-step += (apr_time_now() - curr);
 +if (w-step = wd_interval) {
 +if (!ctx)
 +apr_pool_create(ctx, w-pool);

Is this check really needed? ctx should be always NULL here.

 +w-step = 0;
 +/* Run watchdog step hook */
 +ap_run_watchdog_step(wd_server_conf-s, w-name, ctx);
 +}
 +}
 +if (ctx)
 +apr_pool_destroy(ctx);
 +if (!w-is_running) {
 +break;
 +}
 +}
 +

Re: svn commit: r741947 - in /httpd/httpd/trunk/modules/mappers: config9.m4 mod_watchdog.c mod_watchdog.h

2009-02-08 Thread Ruediger Pluem


On 02/07/2009 08:51 PM, mt...@apache.org wrote:
 Author: mturk
 Date: Sat Feb  7 19:51:52 2009
 New Revision: 741947
 
 URL: http://svn.apache.org/viewvc?rev=741947view=rev
 Log:
 Add watchdog module
 
 Added:
 httpd/httpd/trunk/modules/mappers/mod_watchdog.c   (with props)
 httpd/httpd/trunk/modules/mappers/mod_watchdog.h   (with props)
 Modified:
 httpd/httpd/trunk/modules/mappers/config9.m4

Minor nit:

mod_watchdog.c: In function 'wd_post_config_hook':
mod_watchdog.c:426: warning: unused variable 'lk'


Regards

RĂ¼diger



Re: svn commit: r741947 - in /httpd/httpd/trunk/modules/mappers: config9.m4 mod_watchdog.c mod_watchdog.h

2009-02-08 Thread Mladen Turk

Ruediger Pluem wrote:



+}
+if (w-is_running  w-callbacks == NULL) {
+/* This is hook mode watchdog
+ * running on WatchogInterval
+ */
+w-step += (apr_time_now() - curr);
+if (w-step = wd_interval) {
+if (!ctx)
+apr_pool_create(ctx, w-pool);


Is this check really needed? ctx should be always NULL here.



Yes, it's NULL, but only for the first callback.
If there are multiple callbacks they will all use
the same pool for this call.
Also, pool gets created only if one of the callbacks
actually times out.


+
+/* This mutex fixes problems with a fast start/fast end, where the pool
+ * cleanup was being invoked before the thread completely spawned.
+ */


Can you elaborate this a bit more (maybe with an example)?
I currently do not understand which problem is solved here.



Well, I reused the mod_heartbeat code here, and I think it's
very useful to ensure the thread actually gets created and
running before we allow to kill the child.


+apr_thread_mutex_lock(w-startup);
+
+/* Start the newly created watchdog */
+rc = apr_thread_create(w-thread, NULL, wd_worker, w, p);
+if (rc) {
+apr_pool_cleanup_kill(p, w, wd_worker_cleanup);


I don't see that this cleanup gets registered anywhere.



That got slipped during copy/paste from my dev branch.
Fixed, thanks.


Regards
--
^(TM)