On Thursday 02 August 2007 21:11, Dossy Shiobara wrote:

> Tom, you're a "project admin" of the AOLserver project at SourceForge
> and have been for years, now.  You've had CVS commit access for as long
> as I can remember.  The only commit I can find from you
> ([EMAIL PROTECTED]) *ever* is this one:
...
> This appears to be a change first made in the NaviServer project (see:
> nsd/tclresp.c 1.12).  Reviewing the diff of your change, we see:
...
> Considering interps aren't shared between threads, exactly what "shared
> object" is your commit log referring to, here?  I mostly ignored asking
> this question when the commit first happened, but I'm now curious: where
> did this contribution originate?

Nice you bring that up Dossy. I had to look back at my emails on the subject, 
which went to the AOLserver list. This documents one of the first Butt Fucks 
delivered without warning: changing ns_atclose to only work in a connection 
thread.  

Since you asked I'll include my previous emails here, note the complete 
silence from you or anyone at AOL:

----------------------
Date: 
01/21/07 11:24 am
I have been getting some crashes in AOLserver (current cvs version).
AOLserver doesn't exit, but prints the following and stops responding:

'Tcl_SetBooleanObj called with shared object'

Here is a tcl page which exposes the behavior:

-----------
# Script to expose bug with ns_atclose/namespace commands
set store "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789abcdefghijklmnop"
namespace eval ::bug { }

# Commenting out this line leads to bug: 'Tcl_SetBooleanObj called with shared 
object'
#namespace eval ::bug::$store { }

proc ::bug::atClose { store } {
    ns_log Debug "checking if namespace ::bug::$store exists"
    if {[namespace exists ::bug::${store}]} {
        ns_log Debug "Deleting namespace ::bug::$store"
        namespace delete ::bug::${store}
        #log Notice "Closed store (memory delete) $store"
        return $store
    } else {
        ns_log Debug "namespace ::bug::$store does not exist"
    }

}

# Comment out one of these and things work fine:
ns_atclose ::bug::atClose $store
#ns_atclose ::bug::atClose $store


ns_return 200 text/plain "ns_atclose bug"

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

The bug doesn't show up under all conditions. If the namespace exists, or had 
existed and was deleted, things work as expected. Also, even if the namespace 
never existed, if ns_atclose is only called once, things work as expected.

However, if the namespace to be deleted never existed, and ns_atclose is 
called twice with the same args, none of the ns_log Debug statements print, 
and the crash occurs. (But the page is returned)

Not sure what is the cause.

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

To: 
AOLSERVER@listserv.aol.com
  Date: 
01/21/07 05:06 pm
Okay, some more info on this.

ns_atclose has been changed in some strange ways.

First it now requires that you are in an open connection to invoke ns_atclose.

ns_atclose used to execute in scheduled procs, which makes sense so that you 
can use one method to clean up stuff in case of errors. 

It is easy to re-enable adding ns_atclose to scheduled procs by removing a few 
lines of code. Now I can call ns_atclose everywhere, but in scheduled procs, 
the cleanup scripts don't run.

Question is: why the (silent) change, and
is there something to replace this?

The old description of the command is here:
<http://rmadilo.com/files/nsapi/ns_atclose.html>

I still haven't figured out where exactly the crash is coming from, but _it is 
not in the NsAtCloseObjCmd or NsRunAtClose... code.

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

Date: 
01/21/07 07:17 pm
I found the following change fixes the bug:

in nsd/tclresp.c, line 840:

static int
Result(Tcl_Interp *interp, int result)
{
   /* Tcl_SetBooleanObj(Tcl_GetObjResult(interp), result == NS_OK ? 1 : 0); */
    Tcl_SetObjResult(interp, Tcl_NewBooleanObj((result == NS_OK ? 1 : 0)));
    return TCL_OK;
}

I'll commit the change.

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

Date: 
01/22/07 08:30 am
On Sunday 21 January 2007 23:20, Brett Schwarz wrote:
> That's funny actually...I just changed a bunch of these cases in a Tcl
> extension I help maintain, just earlier today. I happened upon this post
> that talks about it:
> http://sourceforge.net/mailarchive/forum.php?thread_id=30611212&forum_id=43
>966
>
> Might be worthwhile doing an audit of the rest of the aolserver code for
> these occurances.

I only found a few in the AOLserver code, I changed about half before I found 
the one that stopped the bug. 

I even changed one in the tcl codebase that uses this while checking if a 
namespace exists. 

I have a feeling that the bug shows up for some other reason. ns_atclose 
stores scripts and uses a hash array. I'm guessing that two identical scripts 
might appear as one at some point. This could change the reference count for 
the object, somehow leading to the problem.

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

So I'm sorry that you missed out on this bug. But you didn't offer any input, 
nor did anyone else at AOL. Maybe I fixed a crash, but ns_atclose is still 
broken, thanks to yet another smart move by the AOL team.

I still can't figure out how you equate bad code with contribution? And I 
thank you for pointing out that I somehow managed to use AOLserver for a 
decade, releasing many modules, without needing to change any code. Somehow 
you see this as not good.  

But in case there is any remaining question if I ever submitted any other 
code, let me say that yes I did, several times. Unfortunately I made the 
mistake of asking an AOL employee if the patch was okay. Apparently not. I 
got a response that I didn't follow the engineering standards or whatever. So 
I compared the standards with the AOLserver code, prior to my changes, and 
found so many differences that I was unable to figure out just what rule I 
had violated. Who cares that the patch addressed a security issue with 
user/groups, I violated some unspoken rule. 

A second patch, actually a module which required a few small core changes, 
provided code to add default values (used in C code) to the config ns_sets. 
So if someone wished to figure out what the config value was, they could use 
the same method, even if not set in the config file.  What a dumb idea that 
was, making it easy to figure out what values are being used. Who would ever 
want that?

Finally I will just say that apparently the way to get someone's attention, to 
become important, all you need to do here is fuck up some code which works 
perfectly fine. If you don't do that, you are just a griefer.

What an idiot I am, if I only fucked up the source code I would be somebody, 
but since I didn't do that everyone can just ignore me. I'm a complete 
non-entity because I have subtracted two lines of code and added three, 
although I should admit that one of those added lines was just the original 
line commented out, so I have netted ZERO lines. I'm Mr. Zippo! On the other 
hand, these zero lines of code were the subject of six messages and one cvs 
email, so I guess I can't be accused of keeping anyone in the dark about what 
I was doing and why. 

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.

Reply via email to