Re: (ITS#6104) race condition with cancel operation

[hyc@symas.com]

h.b.furuseth@usit.uio.no wrote:
> Well, this turned out a lot of problems.  Currently blocked by
> ITS#6138 Bad Cancel/Abandon/"internal abandon"/Syncprov interactions.
>
> back-relay/op.c done (ITS#6133). syncprov/retcode issues may fit better
> under ITS#6138.
>
> Bitflags should be unnecessary as such, just need a set of values.
> Though it wouldn't hurt if they were chosen so one could use bit
> operations to reduce checks for "o_abandon == A or B".  "Suppress
> response" value mentioned in ITS#6138 also needed, maybe that must be a
> bitflag when combined with Cancel stuff.

Yes, it still seems some bitflags will be more appropriate.

> Some tidbits uglifying a fix:
> * op->o_conn->c_mutex is locked when be->be_abandon() is called,
>    but not when be->be_cancel() is called.
> * send_ldap_ber() needs the lock while it holds&conn->c_write1_mutex,
>    but code elsewhere indicates the reverse lock order.
>
> Question:
>    syncprov_op_abandon() holds&si->si_ops_mutex when setting
>    so->s_op->o_abandon.  Does it need that?  The function needs to
>    grab op->o_conn->c_mutex when called as Cancel, but must not do
>    that while holding&si->si_ops_mutex since Abandon grabs the
>    locks in the opposite order.

syncprov_op_abandon() can remove ops from the list, so yes, it must hold 
si_ops_mutex.

-- 
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/