Re: [PATCH] Make guardians and (ice-9 popen) thread-safe

[Mark H Weaver <mhw@netris.org>]

Hi Ludovic,

Thanks for the review!  I incorporated your (excellent) suggestions, and
pushed it to stable-2.0.

ludo@gnu.org (Ludovic Courtès) writes:
>> From 13fcd175bc31b5df400dd518836bdce32f32206a Mon Sep 17 00:00:00 2001
>> From: Mark H Weaver <mhw@netris.org>
>> Date: Sun, 17 Nov 2013 03:19:32 -0500
>> Subject: [PATCH 2/6] Block system asyncs while 'overrides_lock' is held.
>>
>> * libguile/procprop.c (scm_set_procedure_property_x): Block system
>>   asyncs while overrides_lock is held.  Use dynwind block in case
>>   an exception is thrown.
>
> [...]
>
>> -  scm_i_pthread_mutex_lock (&overrides_lock);
>> +  scm_dynwind_begin (0);
>> +  scm_i_dynwind_pthread_mutex_lock_with_asyncs (&overrides_lock);
>>    props = scm_hashq_ref (overrides, proc, SCM_BOOL_F);
>
> Could you recap why asyncs must be blocked here, and add it as a
> comment?
>
> I would think that there’s no way ‘scm_hashq_ref’ can trigger an async
> run, because it doesn’t call out to Scheme, does it?

That's right, but in 'scm_set_procedure_property_x',
'scm_i_program_properties' is also called while the mutex is locked.
'scm_i_program_properties' calls out to the VM, so asyncs can be run.
I encountered deadlocks from this exact scenario in my previous attempt
to fix (ice-9 popen) thread safety.

I added a comment.

> Also, I wonder if this should rather be exposed as ‘%port-property KEY’
> and ‘%set-port-property! PORT KEY VALUE’.  Those two procedures could
> eventually do any locking required.

Good idea.

    Thanks!
      Mark