On Monday 01 October 2007 10:48, Andrew Piskorski wrote:
> Tom, Gustaf has been both discussing and heavily using this one patch
> for several years. Just how much more discussion do you want? It
> obviously has been working well for U. Wien and others for years now,
> so why not just adopt their proven patch as is, rather than screwing
> around turning it into a loadable module?
Patching core code with experimental features is screwing around. There is
nothing wrong with screwing around with your own copy, but accepting this
type of code into core would lower the bar for any change. This code has
nothing to do with the normal workings of AOLserver. I just spent a few days
looking for any other example of this type of thing. Instead I found some
nice comments like this:
tclsock.c-766- /*
tclsock.c-767- * Pass a dup of the socket to the callback thread, allowing
tclsock.c-768- * this thread's cleanup to close the current socket. It's
tclsock.c-769- * not possible to simply register the channel again with
tclsock.c-770- * a NULL interp because the Tcl channel code is not entirely
tclsock.c:771: * thread safe.
tclsock.c-772- */
tclsock.c-773-
tclsock.c-774- sock = ns_sockdup(sock);
...
tclsock.c-1077- if (cbPtr->chan == NULL) {
tclsock.c-1078- /*
tclsock.c-1079- * Create and register the channel on first use. Because
tclsock.c:1080: * the Tcl channel code is not entirely thread safe, it's
tclsock.c-1081- * not possible for the scheduling thread to create and
tclsock.c-1082- * register the channel.
tclsock.c-1083- */
tclsock.c-1084-
tclsock.c-1085- cbPtr->chan = Tcl_MakeTcpClientChannel((ClientData) sock);
> What's the actual problem with Gustaf's code? You've obviously read
> and thought about it, Tom (which I have not), but so far I see a lot
> of theoretical hand wavy complaints from you, but little solid
> criticism of the actual code.
Both of these are used in the patch, and that doesn't mean there is a problem,
but these dup/copy are happening once per receiving thread. What happens when
multiple channels get pushed into a single thread? You also have to fake up
the nContentSent so that the access.log is 'maybe' correct.
Anyway, my criticism is limited to the fact that you don't need to patch the
core to add this feature, and the change does not at all match up with
anything else in AOLserver core. Is it okay to add anything? How about remove
anything? If Gustaf can add commands, why can't I remove them? Isn't this the
definition of anarchy? There is no logical basis for making a decision, just
the ability to do so. The module concept completely eliminates this problem
by dividing up responsibilities and giving it to those who are interested in
it. The idea that modules have owners (usually folks negotiate with the
maintainer) but the core is a free pass is pretty strange.
The mere fact that the code has been in use for several years does not mean
that this is the way it should go in the public community code.
tom jackson
--
AOLserver - http://www.aolserver.com/
To Remove yourself from this list, simply send an email to <[EMAIL PROTECTED]>
with the
body of "SIGNOFF AOLSERVER" in the email message. You can leave the Subject:
field of your email blank.
