Forum OpenACS Q&A: carnage blender is live on aolserver 4 beta5

Here's what I had to do, if you're curious:

• download & compile nspostgres4 beta. I'm running oacs 3.2, so this was the only extra module I needed. (rl_returnz compiled with no changes.)
• add nsdb to my init.tcl modules list -- otherwise, it doesn't get loaded in 4
• specify -b ip:port in my startup; they broke it so it won't bind before switching to the non-root user otherwise, so port 80 gets permission denied
• patch pthread.c so "ns_mutex destroy" doesn't have the potential of crashing the server. (aolserver developers don't feel this patch is worth applying officially but if you ever want to destroy a mutex, you pretty much need it.)

  --- pthread.c   Sat May  3 19:04:52 2003
  +++ pthread.c.old       Sat May  3 19:03:10 2003
  @@ -181,14 +181,9 @@
   void
   NsLockFree(void *lock)
   {
  -    int err = EBUSY;
  +    int err;
  
  -    /* spinwait until it's unlocked */
  -    while (err == EBUSY) {
  -       err = pthread_mutex_destroy((pthread_mutex_t *) lock);
  -    }
  -    /* {0,EBUSY} is supposed to be the only valid return codes for _destroy,
  -       but we'll check anyway */
  +    err = pthread_mutex_destroy((pthread_mutex_t *) lock);
       if (err != 0) {
          NsThreadFatal("NsLockFree", "pthread_mutex_destroy", err);
       }
  

• patch tclvar.c so "nsv_incr" behaves the way it should: throw an error when the key doesn't exist. I have not yet submitted this patch.

  --- tclvar.c    Sat May  3 20:17:44 2003
  +++ tclvar.c.old        Sat May  3 19:53:02 2003
  @@ -242,7 +242,7 @@
   NsTclNsvIncrObjCmd(ClientData arg, Tcl_Interp *interp, int objc, Tcl_Obj **objv)
   {
       Array *arrayPtr;
  -    int count, current, result;
  +    int count, current, result, new;
       char *value;
       Tcl_HashEntry *hPtr;
  
  @@ -256,23 +256,21 @@
          return TCL_ERROR;
       }
       arrayPtr = LockArray(arg, interp, objv[1], 1);
  -    hPtr = Tcl_FindHashEntry(&arrayPtr->vars, Tcl_GetString(objv[2]));
  -    if (hPtr != NULL) {
  +    hPtr = Tcl_CreateHashEntry(&arrayPtr->vars, Tcl_GetString(objv[2]), &new);
  +    if (new) {
  +       current = 0;
  +       result = TCL_OK;
  +    } else {
          value = Tcl_GetHashValue(hPtr);
          result = Tcl_GetInt(interp, value, ¤t);
  -       if (result == TCL_OK) {
  -           Tcl_Obj *obj = Tcl_GetObjResult(interp);
  -           current += count;
  -           Tcl_SetIntObj(obj, current);
  -           UpdateVar(hPtr, obj);
  -       }
       }
  -    UnlockArray(arrayPtr);
  -
  -    if (hPtr == NULL) {
  -       Tcl_AppendResult(interp, "no such key: ", Tcl_GetString(objv[2]), NULL);
  -       result = TCL_ERROR;
  +    if (result == TCL_OK) {
  +       Tcl_Obj *obj = Tcl_GetObjResult(interp);
  +       current += count;
  +       Tcl_SetIntObj(obj, current);
  +       UpdateVar(hPtr, obj);
       }
  +    UnlockArray(arrayPtr);
       return result;
   }
  

Good news about the stability, cool.

patch pthread.c so "ns_mutex destroy" doesn't have the potential of crashing the server. (aolserver developers don't feel this patch is worth applying officially but if you ever want to destroy a mutex, you pretty much need it.)

"I think it's fine that NsThreadFatal kills nsd - design plan has always been that thread errors are fatal errors, i.e., process must be killed and fixed. As for the thread destory thing, idea is to use proper syncronization (e.g., Ns_ThreadJoin) to ensure when it's possible to destroy a mutex. Not doing so seems like sloppy code."

• there is no interface to check if another thread is using a mutex, so how can it be a programmer error if we don't provide a way to avoid it? (and a quick examination of the pthread functionality should convince you that we don't provide this because it's not possible to do so at that level.)
• even if we jump through totally unnecessary bookkeeping hoops so that thread A, who wants to destroy the mutex, knows the thread id of B, who is currently using it, there is no guarantee that ns_thread wait on B, followed by destroy, would get the mutex before thread C, who is blocking on ns_mutex lock.

setup:
nsv_set mutexes 0 [ns_mutex create]

thread A:
ns_mutex lock [nsv_get mutexes 0]

thread B:
ns_mutex destroy [nsv_get mutexes 0]

poof, there goes your server.

you could try grepping for mutex_destroy in your error log for kicks.

So, did you post this to the AOLserver list, or just send private email to Jim? Jim is not involved in the day to day development of AOLserver, and you might want a wider audience for your concerns. Discussions like this should be on a public list.

I haven't tried it in a single thread either, but lock followed immediately by destroy should also result in a crash in an unpatched nsd. The man page says,

pthread_mutex_destroy destroys a mutex object, freeing the resources it might hold. The mutex must be unlocked on entrance.

nsthread(1175) error: ptread_mutex_destroy failed in NsMutexDestroy: Device or resource busy
Aborted

[On my patched server, I was correct, it loops infinitely.]

(I think you are mistaken about other race conditions...  the pthread code takes care of those nicely.)