tcl-websh-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ronnie Brunner <ron...@netcetera.ch>
Subject Re: remaining fixme's
Date Tue, 26 Mar 2002 15:30:06 GMT
Concerning the interpool.c:502 fixme:

> davidw@dedasys.com (David N. Welton) writes:
> 
> > interpool.c:502:	/* fixme: in threaded mode, this is a bit too rude maybe */
> > 
> > How shall we make it more polite?  I don't really understand the
> > problem - destroyPool might stomp on someone else's resources?
> 
> Unless there are further comments, I don't see a problem with this.

int initPool(websh_server_conf * conf)
{
    static called = 0;

    Tcl_MutexLock(&(conf->webshPoolLock));
    if (called == 1) {
        Tcl_MutexUnlock(&(conf->webshPoolLock));
        return 1;
    } else {
        Tcl_MutexUnlock(&(conf->webshPoolLock));
        called = 1;
    }
    ...
}

There was once a fixme: 

 /* fixme: is this really called once, by a single thread, and we don't do
    any locking in here ??*/

The code above should take care of that locking. Two remarks: the
called = 1 should be before the Tcl_MutexUnlock in the else clause.
Looking at the code I can't see any possibility that initPool could be
called more than once: it's only called from websh_init_child which is
the hook for child initializytion -> this is only called once, unless
Apache does some really strange things. From the http_config.h:

typedef struct module_struct {
    ...

    /* Regardless of the model the server uses for managing "units of
     * execution", i.e. multi-process, multi-threaded, hybrids of those,
     * there is the concept of a "heavy weight process".  That is, a
     * process with its own memory space, file spaces, etc.  This method,
     * child_init, is called once for each heavy-weight process before
     * any requests are served.  Note that no provision is made yet for
     * initialization per light-weight process (i.e. thread).  The
     * parameters passed here are the same as those passed to the global
     * init method above.
     */
#ifdef ULTRIX_BRAIN_DEATH
    void (*child_init) ();
    void (*child_exit) ();
#else
    void (*child_init) (server_rec *, pool *);
    void (*child_exit) (server_rec *, pool *);
#endif

    ...

} module;

I can't imagine any change in Apache 2 for this, although I haven't
found any documentation that supports my thesis.

Long story short conclusion: I'd dump the whole lock.

>From this issue we could conclude that the remaining fixme

     /* fixme: in threaded mode, this is a bit too rude maybe */

can savely be ignored as well (since we're called only once and before
any request is handled)

BTW: we should not even enter this if clause anyway -> we have a
serious problem, if this ever gets called. Suggested fix:

    if (conf->mainInterp != NULL || conf->webshPool != NULL) {
        /* we have to cleanup */
#ifndef APACHE2
	ap_log_printf(conf->server, "initPool: mainInterp or webshPool not NULL\n");
#else /* APACHE2 */
        ap_log_error(APLOG_MARK, APLOG_NOERRNO | APLOG_ERR, 0, conf->server,
	     "initPool: mainInterp or webshPool not NULL\n");
#endif /* APACHE2 */
        destroyPool(conf);
    }

Objections?
------------------------------------------------------------------------
Ronnie Brunner                               ronnie.brunner@netcetera.ch
Netcetera AG, 8040 Zuerich    phone +41 1 247 79 79 Fax: +41 1 247 70 75

---------------------------------------------------------------------
To unsubscribe, e-mail: websh-dev-unsubscribe@tcl.apache.org
For additional commands, e-mail: websh-dev-help@tcl.apache.org


Mime
View raw message