Re: srfi-18 requirements

[Neil Jerram]

"Julian Graham" <address@hidden> writes:

> Hi Neil,
>
>
>> Would that also mean that you could revert the change to the spawning
>> of signal_delivery_thread()?
>
> Of course.

Good, thanks.

>> As this stands, I'm concerned that
>> you're introducing an observable difference.  For example: program
>> calls sigaction specifying a signal handler and a specific thread;
>> later that thread exits, then the signal is raised.  I believe this
>> will cause signal_delivery_thread/scm_system_async_mark_for_thread to
>> raise a "thread has already exited" exception, which is currently
>> reported.
>
> Yeah, it's "reported," but you can't do anything about it
> programmatically, so all you can really do is observe it.  But, yes,
> point taken.

Thanks.

>> No, I meant how the srfi-1 map (defined by module (srfi srfi-1)) is
>> distinct from the core Guile map (defined by (guile)).
>>
>> In other words, the suggestion is that the SRFI-18 implementation of
>> join-thread would not be a core binding, but would come from (srfi
>> srfi-18).
>
> Well, maybe.  Except that I don't really see the benefit to thread API
> users who weren't depending on idiosyncratic threading behavior.  And
> it seems to me that SRFI-1 had more behavioral conflict with Guile

(I don't want to go back to whatever we did for SRFI-1; let's look at
this patch and SRFI-18 on their own merits.)

> primitives than does SRFI-18, so if SRFI-18 functionality can be
> introduced by extending the core rather than providing a parallel
> implementation, then there'll be fewer tears for everyone.

I have what I think is a clearer view on this now, now that I
understand more about what's happening in this enhancement.

Logically speaking I think this patch is a combination of 3 things.

1. Bug fixes and improvements (like admin_mutex) to the existing
   thread code.

2. Enhancements to the set of core thread primitives, to allow
  - timed locking and unlocking of mutexes
  - timed joining
  - defined handling of abandoned mutexes
  - new predicates for threads, mutexes and condition variables.

3. SRFI-18 APIs and specific behaviour (notably with exceptions and
   joining).

We've already agreed to split out and apply (1) first; no change
there, except please also include improvements like the use of
admin_mutex, if you agree that that makes sense.  (I will filter the
latter out when applying to 1.8.x, as they aren't strictly needed
there.)

(2) and (3) are currently intertwined, and I think the key to
clarifying this patch is to imagine a clear boundary between them.  In
other words,

- first (2) we enhance the thread primitives - in ways that are useful
  for SRFI-18, but not _only_ for SRFI-18 thread code;

- then (3) we write the SRFI-18 API.

I've said more about the specific changes to achieve this below.  If
you agree with them, one of the advantages of this is that a lot of
the code that is currently part of the C patch can be moved into (srfi
srfi-18), in Scheme.  We end up with all of (3) being in Scheme -
which is great, because it then makes perfect sense to say that

- all of the C changes are generic enhancements, so should be in core
  Guile, so there is no argument or need for a separate SRFI-18 C
  library

- any issues about when the SRFI-18 API is used (as opposed to the
  core Guile API) are covered in the usual way by the module system.

So, to be precise about the things that are currently in the C patch,
but which I think should not be...

* thread exception, exception_preserve_catch_handler,
  join_thread_timed

The only enhancement needed in the core is for a join-thread proc that
takes a timeout arg, and which can report somehow whether it timed
out.

The SRFI-18 behaviour of throwing a timed-out exception can be
implemented in Scheme.

The storage and later retrieval of a thread exception can be
implemented in Scheme like this:

 (define thread->exception (make-object-property))

 (define (srfi-18-exception-preserver key . args)
   (if (or (srfi-18-terminated-thread-exception? key args)
           (srfi-18-uncaught-exception? key args))
       (set! (thread->exception (current-thread)) (cons key args))))

 When the SRFI-18 Scheme code creates a thread, it calls
 call-with-new-thread with srfi-18-exception-preserver as the
 thread-handler arg.

 The SRFI-18 version of join-thread can (i) call the core join-thread
 proc; (ii) if it returns successfully, access (thread->exception
 JOINED-THREAD), and behave accordingly.

That leaves one loose end in the C patch, namely the use of
exception_preserve_catch_handler() in do_thread_exit().  SRFI-18 has
no concept of thread cleanup proc, so I don't think the SRFI-18
exception preservation semantics should or need to apply to the thread
cleanup code.  So I think this change can simply be reverted.  (Any
thread cleanup proc is of course free to protect itself however it
wishes, and even use srfi-18-exception-preserver if that makes sense.)

Note that this also allows us to remove the SRFI-18/34 symbols from
the C code.

* mutex state

A confusing element here is the bizarre locked/not-owned state.

AFAICS, SRFI-18 specifies nothing at all (apart from mutex-state
itself) which depends on the difference between locked/owned and
locked/not-owned.  Therefore I don't think we should support this
state in the core.

It can be supported in (srfi srfi-18) like this:

(define mutex->not-owned? (make-object-property))

(define (srfi-18-mutex-lock mutex . optargs)
  (apply core-mutex-lock mutex optargs)
  (if (and (= (length optargs) 2)
           (eq? (cadr optargs) #f))
      ;; Support the locked/not-owned state.
      (set! (mutex->not-owned? mutex) #t)))

Then SRFI-18 mutex-state can be implemented in Scheme also:

(define (mutex-state m)
  (let ((thread (mutex-owner m)))
    (cond ((not thread) 'not-abandoned)
          ((thread-exited? thread) 'abandoned)
          ((mutex->not-owned? m) 'not-owned)
          (else thread))))

And the #f/SCM_UNDEFINED changes can be reverted.

That just leaves the uses of fat_mutex_state() in fat_mutex_lock().  I
personally find that these uses obfuscate what has changed, and would
prefer if the code went back to checking m->owner as it did before.
Is that feasible?

Note that this also allows us to remove the SRFI-18 state symbols from
the C code.

* (end of things to remove from the C patch)

Finally, there are quite a few spurious changes (or perhaps just that
I don't understand yet) in patch: whitespace, line break and docstring
changes.  Can you revert all these, as they only confuse the overall
picture?

To conclude...

I'm sorry that I'm asking for some significant changes here, but I
hope that you'll agree that they make the enhancement clearer, and in
particular that it is a good thing to reduce the changes that we need
to make to the C code.  Please let me know what you think.

Regards,
        Neil