Re: How to destroy a condvar(9) with waiters?

[Stephan <stephanwib%googlemail.com@localhost>]

I think I found another approach. The destruction is now split in two parts:

1) The destroyer changes the state to destroyed and removes the object
from the global list.
2) Each waiter checks the state and waiters count of the object. If
the state is "destroyed" and the waiters count equals zero, a
"delete_physical()" function is called which destroys the CVs,
releases and destroys the interlock and finally frees all memory.

The waiters part looks like this:

port->kp_waiters++;
error = cv_timedwait_sig(&port->kp_rdcv, &port->kp_interlock, t);
port->kp_waiters--;
if ((port->kp_state == kp_deleted) && (port->waiters == 0)) { /* port
has been logically destroyed, and we are the last waiter */
  kport_delete_physical(port);
  return ENOENT;
}

Can this work?

2015-12-29 7:29 GMT+01:00 Stephan <stephanwib%googlemail.com@localhost>:
> Thanks for your explanation. There are 2 condvars for the object - as
> far as I understand should one refcount be enough to make sure there
> are no waiters left on both, correct?
>
> 2015-12-28 15:33 GMT+01:00 Taylor R Campbell
> <campbell+netbsd-tech-kern%mumble.net@localhost>:
>>    Date: Mon, 28 Dec 2015 14:35:14 +0100
>>    From: Stephan <stephanwib%googlemail.com@localhost>
>>
>>    given there is an object with the following
>>
>>    -an interlock (mutex)
>>    -a condvar(9)
>>    -a "state" variable which can be set to something like state_deleted
>>
>>    [...]
>>
>>    How can I safely destroy the condvar and interlock mutex?
>>
>> In addition to the state, add a reference count to your objects:
>> increment and decrement it in the waiter code, and wait for it to
>> drain to zero in the destroyer code.  Changing the state to deleted
>> tells waiters they need to stop; changing the reference count from
>> nonzero to zero tells the destroyer that the waiters have all stopped.
>> The destroyer will not wake until the last waiter notifies it and
>> releases the lock.
>>
>>
>> /* Waiter: bump reference count, wait, drop reference count.  */
>> if (port->kp_refcnt == UINT_MAX) {
>>         error = EBUSY;
>>         goto fail;
>> }
>> port->kp_refcnt++;
>> while (!ready(port)) {
>>         error = cv_timedwait_sig(&port->kp_rdcv, &port->kp_interlock,
>>             mstohz(timeout));
>>         if (error || (port->kp_state == state_deleted))
>>                 break;
>> }
>> if (--port->kp_refcnt == 0)
>>         cv_broadcast(&port->kp_rdcv);
>>
>>
>> /* Destroyer: remove from list, mark deleted, wait for waiters to drain.  */
>> mutex_enter(&port_list_lock);
>> LIST_REMOVE(port, kp_list);
>> mutex_exit(&port_list_lock);
>>
>> mutex_enter(&port->kp_interlock);
>> port->kp_state = state_deleted;
>> cv_broadcast(&port->kp_rdcv);
>> while (port->kp_refcnt != 0)
>>         cv_wait(&port->kp_rdcv, &port->kp_interlock);
>> mutex_exit(&port->kp_interlock);
>>
>> /* Destroyer now has exclusive reference to port.  */
>> mutex_destroy(&port->kp_interlock);
>> cv_destroy(&port->kp_rdcv);
>> ...
>> kmem_free(port, sizeof(*port));
>>
>>
>> (You can use a different condvar to notify the destroyer if you like,
>> but there's not much benefit to that -- it takes extra space and only
>> avoids a negligible number of spurious wakeups, which you have to deal
>> with anyway.)