Re: srfi-18 requirements

[Neil Jerram <neil@ossau.uklinux.net>]

"Julian Graham" <joolean@gmail.com> writes:

>> >  @c begin (texi-doc-string "guile" "join-thread")
>> > -@deffn {Scheme Procedure} join-thread thread
>> > +@deffn {Scheme Procedure} join-thread thread [timeout]
>> >  @deffnx {C Function} scm_join_thread (thread)
>> > +@deffnx {C Function} scm_join_thread_timed (thread, timeout)
>>
>> Didn't we agree to add a timeout-val parameter here?
>
> No, we didn't, although I agree such a parameter would be pretty
> useful.

Well we discussed it a bit here:
http://lists.gnu.org/archive/html/guile-devel/2008-02/msg00004.html
http://lists.gnu.org/archive/html/guile-devel/2008-02/msg00005.html

>  I'll add that in the next revision I send you.

Cool, thanks.

>> > +       else if (!first_iteration)
>> > +         {
>> > +           if (timeout != NULL)
>> > +             {
>> > +               gettimeofday (&current_time, NULL);
>> > +               if (current_time.tv_sec > timeout->tv_sec ||
>> > +                   (current_time.tv_sec == timeout->tv_sec &&
>> > +                    current_time.tv_usec * 1000 > timeout->tv_nsec))
>> > +                 {
>> > +                   *ret = 0;
>> > +                   break;
>> > +                 }
>>
>> Is timeout an absolute time, or relative to when join-thread was
>> called?  Before getting to this code, I thought it was relative - but
>> then I don't see how the code above can be correct, because it is
>> comparing against the absolute gettimeofday() ...?
>
> It's absolute -- like the arguments for the existing timed
> synchronization primitives

OK, yes, I see now.  The code is fine as it stands, then.

> (and like the timed parts of the SRFI-18 API).  (Unless I'm
> mistaken...)

But that's not completely right.  SRFI-18 says that timeout-val can be
one of the following:

    * a time object represents an absolute point in time
    * an exact or inexact real number represents a relative time in seconds from the moment the primitive was called
    * #f means that there is no timeout 

So for the SRFI-18 API, timeout-val is sometimes absolute and
sometimes relative!  I guess that just means that the SRFI-18 Scheme
code will have to add (current-time), when an integer or float is
given to it.

>
>> > -static char *
>> > -fat_mutex_unlock (fat_mutex *m)
>> > +static void
>> > +fat_mutex_unlock (SCM mx)
>> >  {
>> > -  char *msg = NULL;
>> > -
>> > +  fat_mutex *m = SCM_MUTEX_DATA (mx);
>> >    scm_i_scm_pthread_mutex_lock (&m->lock);
>> > -  if (!scm_is_eq (m->owner, scm_current_thread ()))
>> > +  if (m->level > 0)
>> > +    m->level--;
>> > +  else
>>
>> It looks like there is a significant change to the semantics here: any
>> thread can unlock a mutex, not just the thread that locked it.  Is
>> that the intention, or am I misunderstanding?
>
> No, that's the intention (it's explicitly permitted by SRFI-18).  I
> thought you were okay with that, since it was not on your list of
> stuff that didn't belong in C.  If that's too big of a change, might I
> suggest we add a function that forcibly unlocks a mutex, regardless of
> the owner?

Sorry for missing this before.  The SRFI-18 semantics are really
interesting, but I think we need to preserve the existing semantics
too for back-compatibility.  i.e. we need to preserve the two
conditions described by this existing code:

  if (!scm_is_eq (m->owner, scm_current_thread ()))
    {
      if (scm_is_false (m->owner))
	msg = "mutex not locked";
      else
	msg = "mutex not locked by current thread";
    }

I guess that means that scm_unlock_mutex_timed will need to take
another optional parameter (or two) indicating whether

- it is an error to unlock an unlocked mutex (default yes, but SRFI-18
  will pass "no")

- it is an error to unlock a mutex owned by another thread (default
  yes, SRFI-18 will pass "no").

Can you propose a representation for this?

>> Actually, that strongly says to me that we don't need the `cond' part
>> of this API to be implemented in C.  Can we move that to the SRFI-18
>> Scheme code, and leave the C API as a plain unlock-mutex operation?
>
> Fine by me (again. left this one in because you didn't squawk about it
> earlier), except that it might be harder to guarantee the safety of
> mixing the mutex and cond passed to the SRFI-18 Scheme implementation
> with non-SRFI-18 calls -- C generally provides a convenient protection
> against deadlock for things like that.

I'm not sure about that argument, but I think it's moot anyway -
because I think the current implementation, which equates to

  (begin
    (wait-condition-variable cond-var mutex)
    (unlock-mutex mutex))

does not always behave as SRFI-18 says.  Specifically, if there is
another thread trying to lock `mutex', `(wait-condition-variable
cond-var mutex)' may block, after the cond-var has been signalled,
because it is not able to reacquire the mutex.  Whereas SRFI-18 says
that the thread that calls mutex-unlock! "can unblock at any time, but
no later than when an appropriate call to condition-variable-signal!
or condition-variable-broadcast! is performed (see below), and no
later than the timeout (if timeout is supplied)".

Given the definitions of `wait-condition-variable' and SRFI-18's
`mutex-unlock!', and that we want Guile to provide both of these, it
seems to me now that `mutex-unlock!' is actually the more primitive
operation, and that `wait-condition-variable' could be written as

    scm_unlock_mutex_timed (mx, cv, 0);
    scm_lock_mutex (mx;)

Is it possible to reorganize the relevant code a bit, so that
scm_unlock_mutex_timed (mx, cv, 0) does not lock and immediately
unlock the mutex after the cond var has been signalled?

Regards,
     Neil