Re: srfi-18 requirements

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

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

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

>> > Let me know if I've missed anything.
>>
>> I don't think so, and I plan to apply this very soon.  I've found a
>> reliable recipe for reproducing the critical section problem: if a
>> scm_i_gc call is added to make_jmpbuf (), like this:
>
>
> Excellent -- I'll let you know if I can think of a deterministic way
> to reproduce that from user code.

Thanks.  I've decided to give up on this for now, and just check in
the fix.

I was playing with code like the attached patch - which I think is
already more effort than it's worth spending to be able to regressibly
test this :-) - and then I found that even this doesn't work reliably.
To be precise, it hangs every time when run under GDB, but not when
run outside GDB.

So I think the wise course is to accept that there are some things we
can't write tests for.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: test-gc-deadlock.patch --]
[-- Type: text/x-diff, Size: 3387 bytes --]

Index: libguile/debug.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/debug.c,v
retrieving revision 1.120.2.1
diff -u -r1.120.2.1 debug.c
--- libguile/debug.c	12 Feb 2006 13:42:51 -0000	1.120.2.1
+++ libguile/debug.c	6 Feb 2008 23:52:02 -0000
@@ -523,6 +523,30 @@
 
 \f
 
+unsigned scm_forced_internals[1] = { 0 };
+
+SCM_GLOBAL_SYMBOL (scm_sym_gc_in_make_jmpbuf, "gc-in-make_jmpbuf");
+
+SCM_DEFINE (scm_force_internal, "force-internal", 1, 0, 0,
+	    (SCM op),
+	    "Force the next internal Guile operation @var{op} to occur (for test purposes only).")
+#define FUNC_NAME s_scm_force_internal
+{
+  int op_index;
+
+  if (scm_is_eq (op, scm_sym_gc_in_make_jmpbuf))
+    op_index = SCM_FI_GC_IN_MAKE_JMPBUF;
+  else
+    SCM_WRONG_TYPE_ARG(1, op);
+
+  scm_forced_internals[op_index] = 2;
+
+  return SCM_UNSPECIFIED;
+}
+#undef FUNC_NAME
+
+\f
+
 void
 scm_init_debug ()
 {
Index: libguile/debug.h
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/debug.h,v
retrieving revision 1.58
diff -u -r1.58 debug.h
--- libguile/debug.h	4 Nov 2005 21:20:24 -0000	1.58
+++ libguile/debug.h	6 Feb 2008 23:52:02 -0000
@@ -173,6 +173,12 @@
 SCM_API SCM scm_debug_hang (SCM obj);
 #endif /*GUILE_DEBUG*/
 
+#define SCM_FI_GC_IN_MAKE_JMPBUF 0
+extern unsigned scm_forced_internals[];
+#define SCM_FORCE_INTERNAL(OP) ((scm_forced_internals[OP]) && !(--scm_forced_internals[OP]))
+
+SCM_API SCM scm_force_internal (SCM op);
+
 #if SCM_ENABLE_DEPRECATED == 1
 
 #define CHECK_ENTRY      scm_check_entry_p
Index: libguile/throw.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/throw.c,v
retrieving revision 1.108.2.3
diff -u -r1.108.2.3 throw.c
--- libguile/throw.c	2 Jun 2006 23:39:12 -0000	1.108.2.3
+++ libguile/throw.c	6 Feb 2008 23:52:02 -0000
@@ -73,6 +73,8 @@
   SCM answer;
   SCM_CRITICAL_SECTION_START;
   {
+    if (SCM_FORCE_INTERNAL (SCM_FI_GC_IN_MAKE_JMPBUF))
+      scm_i_gc ("test");
     SCM_NEWSMOB2 (answer, tc16_jmpbuffer, 0, 0);
     SETJBJMPBUF(answer, (jmp_buf *)0);
     DEACTIVATEJB(answer);
Index: test-suite/standalone/Makefile.am
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/test-suite/standalone/Makefile.am,v
retrieving revision 1.13.2.7
diff -u -r1.13.2.7 Makefile.am
--- test-suite/standalone/Makefile.am	1 Feb 2008 22:47:51 -0000	1.13.2.7
+++ test-suite/standalone/Makefile.am	6 Feb 2008 23:52:03 -0000
@@ -55,6 +55,9 @@
 check_SCRIPTS += test-bad-identifiers
 TESTS += test-bad-identifiers
 
+check_SCRIPTS += test-gc-in-make-jmpbuf
+TESTS += test-gc-in-make-jmpbuf
+
 # test-num2integral
 test_num2integral_SOURCES = test-num2integral.c
 test_num2integral_CFLAGS = ${test_cflags}
Index: test-suite/standalone/test-gc-in-make-jmpbuf
===================================================================
RCS file: test-suite/standalone/test-gc-in-make-jmpbuf
diff -N test-suite/standalone/test-gc-in-make-jmpbuf
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ test-suite/standalone/test-gc-in-make-jmpbuf	6 Feb 2008 23:52:03 -0000
@@ -0,0 +1,10 @@
+#!/bin/sh
+exec guile -s "$0" "$@"
+!#
+
+(force-internal 'gc-in-make_jmpbuf)
+(system* "sleep" "1")
+
+;; Local Variables:
+;; mode: scheme
+;; End:

[-- Attachment #3: Type: text/plain, Size: 1810 bytes --]


> At any rate, let me pop a few frames off the stack.  The two remaining
> changes for the core features patch were providing a join_thread that
> could indicate a timeout without throwing an SRFI-18 exception; and
> providing a well-defined fat_mutex_lock that doesn't rely on the
> SRFI-18 notion of mutex state.  Two questions:
>
> * What would be an appropriate way for join_thread to indicate a
> timeout?  Given that it's a primitive that can be called from Guile, I
> take it that the standard C approach of passing a pointer to a flag is
> out of the question.  Would it be good enough to have it return #f on
> timeout (even if that leaves some amiguity about whether there was a
> timeout or just a lack of a thread return value)?  Is there a core
> exception it could throw?

How about if the core join-thread takes an optional timeout-val
parameter, like SRFI-18 thread-join! ?  If no timeout-val was
supplied, and the join timed out, the core join-thread would return
#f.

The #f would indeed be ambiguous, but any given caller can eliminate
the ambiguity if they choose to by specifying a timeout-val.

Note that thread-join! can map onto this, in the case where _it_ gets
no timeout-val, by constructing a unique object such as (list
'timeout) and passing this as the timeout-val to the core join-thread.

> * What should be the behavior of fat_mutex_lock when attempting to
> lock an abandoned mutex -- in your earlier email, you seemed amenable
> to the parts of SRFI-18 that shore up some of the poorly-defined
> threading behavior in core threads.  So should locking an abandoned
> mutex be an error?  If so, what kind?  Or should locking an abandoned
> mutex not be an error at all unless you do it using the SRFI-18 API?

I'll get back to you on this one tomorrow!

Regards,
        Neil