Bugs item #449744, was opened at 2001-08-10 02:02
You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=103152&aid=449744&group_id=3152

Category: Architecture: Server (nsd)
Group: None
Status: Open
Resolution: None
>Priority: 7
Submitted By: Jerry Asher (jerryasher)
Assigned to: Nobody/Anonymous (nobody)
Summary: conn driver freeproc never called

Initial Comment:
Communications drivers can have an Ns_ConnFreeProc.

Although it is not quite clear what this procedure
does, the legacy nsunix code choose to use it to free
various buffers, specifically, the conn input buffer.

But no one ever calls these freeing procedures.

My suggestion: define a routine similar to
conn.c/Ns_ConnClose (called Ns_ConnFree...) that calls
connPtr->drvPtr->freeProc (if it's not null).

Where to call it though?  I am not certain.

I chose to call it in in serv.c/ConnRun, as the second
to last statement, right after NsRunCleanups and right
before the dstring is freed.

Is this the right place?

----------------------------------------------------------------------

Comment By: Jerry Asher (jerryasher)
Date: 2001-08-27 08:20

Message:
Logged In: YES
user_id=20647

This patch contains fixes for four bugs:

1.  Memory leak by any communication driver that implements
a driver
    free proc: the bug is that this freeproc is never
called within
    AOLserver.

    Create conn.c/Ns_ConnFree to provide a connection a way
to call
    the driver free proc (supported in registration of a
driver, but
    never called within AOLserver.)

    Call Ns_ConnFree in serv.c/ConnRun at end of
connection.  Also
    call Ns_ConnFree within drv.c whenever there is a
problem queueing
    connection.

    Declare Ns_ConnFree within ns.h

2.  in nsd/drv.c/RunDriver, there are three bugs present in
which well
    defined functions are called with the wrong
structures.  (Or
    called before the structures have been initialized.)
Call it the
    curse of casting, right?  No compiler warnings as the
*wrong*
    structures are correctly cast into the right types.

    To see this, the best way is to compare how similar
routines are
    called or defined by the nssock driver.

Claim 1: In the excerpt
    dPtr = arg;
    dData = dPtr->drvData;
    if (dPtr->locationProc != NULL) {
        loc = (*dPtr->locationProc)(dData);
     . . .
    while ((status = ((*dPtr->acceptProc)(dData, &cData)))
== NS_OK) {

A locationProc is supposed to take a connection structure,
if so, then
locationProc is being called before the accept, and so the
connection
data (presumably dData) has not yet been initialized.
(I'll make the
claim that dData is not necessarily the connection
structure that
locationproc needs, but regardless of whether it is or
isn't, it's not
fair to ask locationproc to crawl down the connection
structure before
the connection has been accepted.)

Here is how ns.h defines the locationProc:
typedef char   *(Ns_ConnLocationProc) (void *pConnCtx);

And for comparison, here is how nssock.cpp implements it's
location
proc (this is comparison only, note that drv.c doesn't
actually call
into nssock/SockLocation since nssock supplies it's own
thread).

static char *
SockLocation(void *arg)
{
    Conn   *connPtr = arg;

This affects drivers that use the drv.c/RunDriver, which
nssock
doesn't (and nsunix does.)  (Near as I can tell, nsunix is
the only
user of this routine in what used to be core AOLserver,
does anyone
else use drv.c/RunDriver?)

The fix for this is to not call locationProc at this
moment -- it's use is
only to generate a log message.


Claim 2.  In the excerpt:

    while ((status = ((*dPtr->acceptProc)(dData, &cData)))
== NS_OK) {
        if (Ns_QueueConn(dData, cData) != NS_OK) {
           (*dPtr->closeProc)(dData);
        }
    }

Ns_QueueConn has been called with the wrong parameters.  It
should be called as:
        if (Ns_QueueConn(dPtr, cData) != NS_OK) {

in nsd/serv.c we can see how Ns_QueueConn is defined:

int
Ns_QueueConn(void *drvPtr, void *drvData)

The first argument is a pointer to a driver, the second
argument is a
handle to a driver specific data structure (each driver
family sticks
different things in there.)

Claim 3.  In the same excerpt, closeProc should be called
with cData
and not dData.



----------------------------------------------------------------------

You can respond by visiting:
http://sourceforge.net/tracker/?func=detail&atid=103152&aid=449744&group_id=3152

Reply via email to