> Well, here is an acid dump, I'll inspect it in detail, but I'm hoping
> someone will beat me to it (not hard at all, I have to confess):
> 
> rumble# acid /sys/src/9/pc/9pccpuf
> /sys/src/9/pc/9pccpuf:386 plan 9 boot image
> /sys/lib/acid/port
> /sys/lib/acid/386
> 
[ ... ]

This bit looks suspicious to me, but I'm really not an authority and I
may easily be missing something:

> acid: src(0xf0148c8a)
> /sys/src/9/ip/tcp.c:2096
>  2091         if(waserror()){
>  2092                 qunlock(s);
>  2093                 nexterror();
>  2094         }
>  2095         qlock(s);
>>2096          qunlock(tcp);
>  2097 
>  2098         /* fix up window */
>  2099         seg.wnd <<= tcb->rcv.scale;
>  2100 
>  2101         /* every input packet in puts off the keep alive time out */

The source actually says (to be pedantic):

        /* The rest of the input state machine is run with the control block
         * locked and implements the state machine directly out of the RFC.
         * Out-of-band data is ignored - it was always a bad idea.
         */
        tcb = (Tcpctl*)s->ptcl;
        if(waserror()){
                qunlock(s);
                nexterror();
        }
        qlock(s);
        qunlock(tcp);

Now, the qunlock(s) should not precede the qlock(s), this is the first
case in this procedure:

/sys/src/9/ip/tcp.c:1941,2424
void
tcpiput(Proto *tcp, Ipifc*, Block *bp)
{
...
}

and unlocking an unlocked item should not be permitted, right?  If s
(the conversation) is returned locked by either of

/sys/src/9/ip/tcp.c:2048
        s = iphtlook(&tpriv->ht, source, seg.source, dest, seg.dest);

or

/sys/src/9/ip/tcp.c:2081
                s = tcpincoming(s, &seg, source, dest, version);

then the qlock(s) is an issue, but I'm sure that is not the case here.

Finally, locking an item ahead of unlocking another is often cause for
a deadlock, although I understand the possible necessity.

        qlock(s);
        qunlock(tcp);

Even though acid points to a different location, I'd be curious to
have the details above expanded on by somebody familiar with the code.

++L


Reply via email to