1
Fork 0
mirror of https://git.savannah.gnu.org/git/guile.git synced 2025-05-17 22:52:25 +02:00
Commit graph

12610 commits

Author SHA1 Message Date
Andy Wingo
32c8ae2009 rashly bump stack limit to 40k words
* libguile/eval.c (scm_debug_opts): Whereas, today's machines are larger
  than yesterday's; GCC consumes more words per stack frame than it used
  to; and you can get quite some recursion in a halfway-compiled system,
  be it resolved: let's bump up the C stack limit to 40k words (160 kB /
  320 kB, depending on word size).
2008-11-11 23:11:27 +01:00
Andy Wingo
f38624b349 add parsers and unparser for ghil; ,language ghil works now
* module/system/repl/common.scm (repl-print): Slightly refine the meaning
  of "language-printer": a language printer prints an expression of a
  language, not the result of evaluation. `write' prints values.

* module/language/ghil/spec.scm (ghil): Define a language printer, and a
  translator for turning s-expressions (not scheme, mind you) into GHIL.

* module/language/scheme/translate.scm (quote, quasiquote): Add some
  #:keyword action, so that we can (quote #:keywords).

* module/system/base/language.scm (<language>):
* module/system/base/compile.scm (read-file-in): Don't require that a
  language have a read-file; instead error when read-file is called.
  (compile-passes, compile-in): Refactor to call a helper method to turn
  the language + set of options into a set of compiler passes.

* module/system/base/syntax.scm (define-type): Allow the type to be a
  list, with the car being the name and the cdr being keyword options.
  Interpret #:printer as a printer, and pass it down to...
  (define-record): Here.

* module/system/il/ghil.scm (print-ghil, <ghil>): New printer for GHIL,
  yay!
  (parse-ghil, unparse-ghil): New lovely functions. Will document them in
  the manual.
2008-11-11 22:52:24 +01:00
Ludovic Courtès
c9d15b0583 gc-benchmarks: Adapt `gcold.scm' so that if conforms to the framework.
* gc-benchmarks/larceny/gcold.scm (main): Rename to `gcold-benchmark'.
  (GCOld): Fix the order of the predicate and run-maker.
2008-11-11 21:01:40 +01:00
Ludovic Courtès
ecdbb582ff gc-benchmarks: Add `gcold.scm', by Clinger, Hansen et al.
See http://www.ccs.neu.edu/home/will/Twobit/benchmarksAbout.html for
details.
2008-11-11 18:27:24 +01:00
Ludovic Courtès
d7b3f25d5a gc-benchmarks: Add `run-benchmark.scm'. 2008-11-11 18:20:15 +01:00
Ludovic Courtès
e4c8d2a2c3 gc-profile: Show the result of `(version)'.
* gc-benchmarks/gc-profile.scm (main): Show `(version)'.
2008-11-11 18:19:24 +01:00
Ludovic Courtès
857a263e4f Have version' return PACKAGE_VERSION'.
* libguile/version.c (scm_version): Return `PACKAGE_VERSION' instead of
  "MAJOR.MINOR.MICRO".
2008-11-10 23:46:29 +01:00
Ludovic Courtès
613ed506cb Change GUILE_VERSION' to 1.9.0-bdwgc'.
* GUILE-VERSION (GUILE_VERSION): Add `-bdwgc' prefix.
2008-11-10 23:45:28 +01:00
Ludovic Courtès
1f26b531ec Use `pkg-config' to detect BDW-GC at configure-time.
* configure.in: Use `PKG_CHECK_MODULES' to look for libgc.  This only
  works with GC 7.x+, which is what we want anyway.
2008-11-10 23:37:53 +01:00
Ludovic Courtès
59e2102091 gc-benchmarks: Allow the iteration count to be passed to `gc-profile.scm'.
* gc-benchmarks/gc-profile.scm (*iteration-count*): New parameter.
  (run-benchmark): Moved from `twobit-compat.scm'.  Honor
  `*iteration-count*'.
  (%options): Add `--iterations'.
  (show-help): Document it.
  (main): Parameterize `*iteration-count*'.
2008-11-10 22:49:29 +01:00
Ludovic Courtès
c1d1d8247c Import GC benchmarks from Larceny, by Hansen, Clinger, et al.
These GPLv2+-licensed GC benchmarks are available from
http://www.ccs.neu.edu/home/will/GC/sourcecode.html .
2008-11-10 22:37:56 +01:00
Ludovic Courtès
b777f3b64c gc-benchmarks: Add a Larceny/Twobit benchmark compatibility layer.
* gc-benchmarks/gc-profile.scm: Load "twobit-compat.scm".
  (save-directory-excursion, load-larceny-benchmark): New procedures.
  (%options): New variable.
  (show-help, parse-args): New procedures.
  (main): Use `parse-args' and `load-larceny-benchmark'.
2008-11-10 20:28:32 +01:00
Andy Wingo
f698d111b4 remove .cvsignore files 2008-11-10 12:17:18 +01:00
Ludovic Courtès
490cf75094 Work around unintentional retention of modules by the GC.
This reverts par of "Document the failure of `gc.test' wrt. unused modules."
(commit 328efeb9a6.)

* ice-9/boot-9.scm (set-module-eval-closure!): Don't set the `module' property
  on CLOSURE.

* libguile/modules.c (scm_lookup_closure_module): Call `abort ()' to make it
  clear that code that uses the `module' property no longer works.  That code
  is unused anyway.
2008-11-05 22:39:31 +01:00
Ludovic Courtès
00b8057d1f Merge branch 'master' into boehm-demers-weiser-gc
Conflicts:
	libguile/threads.c
2008-11-04 19:07:07 +01:00
Ludovic Courtès
f8e7dfdc53 Remove the SMOB mark procedure for source properties.
* libguile/srcprop.c (srcprops_mark): Remove.
  (scm_init_srcprop): Remove call to `scm_set_smob_mark ()'.
2008-11-02 23:24:10 +01:00
Andy Wingo
2651e3c412 proper printing of thunks, reduced disasm verbosity
* module/system/vm/disasm.scm (disassemble-program): Don't print the
  nargs= nrest= etc line, it's redundant.

* module/system/vm/program.scm (program-bindings-as-lambda-list): If the
  program bindings is null, then that's that.
2008-11-02 01:37:00 +01:00
Andy Wingo
6c3686ea14 define macros before functions using macros; more MV fixen in srfi-69
* module/srfi/srfi-69.scm: Move the macros up before the functions that
  use them, so that the compiler can do its job.
  (hash-table-walk): While it is true about what I said about R5RS
  before, it seems that for R6 this will have to change. Anyway. In the
  meantime, since the test suite checks that hash-table-walk procedures'
  return values and number of return values are ignored, call that
  procedure within a call-with-values.
2008-11-01 20:31:57 +01:00
Andy Wingo
9b10d0bcfd fix for (apply values '(1))
* libguile/vm-i-system.c (return/values): In the
  multiple-values-to-a-single-value-continuation (or MV but where N=1),
  null out the correct number of values from the stack. Fixes aborts on
  (apply values '(1)).

* testsuite/t-values.scm (call-with-values): Add a test.
2008-11-01 18:37:48 +01:00
Andy Wingo
42906d7406 fix multiple values coming from interpreted or C procedures
* libguile/vm-i-system.c (call, goto/args): Handle the case in which a
  non-program (i.e. interpreted program or a subr) returns multiple
  values.

* testsuite/t-values.scm: Add test case that exhibited this problem.
2008-11-01 18:19:19 +01:00
Andy Wingo
3fd8807eab make-procedure-with-setter inherits name from getter
* libguile/procs.c (scm_make_procedure_with_setter): Patch through the
  getter's procedure name to the procedure-with-setter. Fixes part of the
  srfi-17 test, as the VM doesn't set procedure-name on define -- but
  perhaps that is the bug that should be fixed. In any case this patching
  is cheap.

* test-suite/tests/eval.test: Change so that (define name pws) is
  initially passed an anonymous procedure-with-setter, as was the case
  before the procs.c change.
2008-11-01 17:12:23 +01:00
Andy Wingo
0a283d1b0b avoid delivering 0 values to 1-valued continuations in srfi-19
* module/srfi/srfi-19.scm: Some parts of this code used a strange idiom,
  `(values)', to indicate that a procedure did nothing. However, quoth
  R5RS:

     Except for continuations created by the `call-with-values'
     procedure, all continuations take exactly one value.

  Indeed the VM indicated this error. I reworked the code to avoid these
  cases.
2008-11-01 14:28:24 +01:00
Andy Wingo
ea93465de7 move scm srfi files to module/srfi, and compile them.
* .gitignore: Add gdb-pre-inst-guile.

* configure.in: Add module/srfi/Makefile.

* module/Makefile.am: Add srfi/.

* module/srfi/: SRFI scheme files moved here, and compiled.

* srfi/Makefile.am: Remove the bits about the scheme files.
2008-11-01 13:49:23 +01:00
Andy Wingo
3f0bce1e14 move guilec.mk to am/guilec
* am/Makefile.am:
* am/guilec: guilec moved here from /guilec.mk.

All includers of guilec adapted.
2008-11-01 13:05:10 +01:00
Andy Wingo
b1368a4166 remove stale env script, clean up gdb-pre-inst-guile
* env: Removed (a vestige of the guile-vm merge).

* gdb-pre-inst-guile.in: Make a bit more robust (using libtool
  --mode=execute).
2008-11-01 12:53:37 +01:00
Andy Wingo
00d0489205 move ice-9/ and oop/ under module/
Moved ice-9/ and oop/ under module/, with the idea being that we have
only scheme under module/. Adjusted configure.in and Makefile.am
appropriately. Put oop/ at the end of the compilation order.
2008-11-01 12:44:21 +01:00
Ludovic Courtès
627796347f Fix initialization of the SMOB GC "kind".
* libguile/smob.c (scm_smob_prehistory): When initializing
  SMOB_GC_KIND, pass 1 as the CLEAR_NEW_OBJECTS argument to
  `GC_new_kind ()'.  Without this, an assertion failure is
  triggered in libgc's `reclaim.c'.
2008-10-31 21:55:55 +01:00
Andy Wingo
5192c9e89b compile goops accessors. woot!
* oop/goops.scm: Define compiler hooks for dealing with @slot-ref and
  @slot-set!.
  (make-bound-check-get, make-get, make-set): Compile these indexed
  accessors instead of having them be closures. Probably slower for the
  memoizer, but faster for the vm... not sure what the perfect solution
  is.

* test-suite/tests/goops.test ("defining classes"): Add a test that
  defining a class with accessors works (it didn't until I figured out
  that (@ (system base compile) compile) thing).
2008-10-31 18:30:27 +01:00
Andy Wingo
1e4b834ab1 new ops: slot-ref, slot-set. remove and recompile your .go files
* libguile/vm-i-scheme.c (slot-ref, slot-set): New ops.
2008-10-31 14:09:01 +01:00
Andy Wingo
eb5f05c320 fix bug in define-scheme-translator
* module/language/scheme/translate.scm (define-scheme-translator): Fix a
  bug in this macro for the syntax-error case.
2008-10-31 14:07:11 +01:00
Andy Wingo
03fa04dfe1 pass backtraces through the compiler
* module/system/base/compile.scm (call-with-nonlocal-exit-protect): New
  helper, like unwind-protect but only for nonlocal exits.
  (call-with-output-file/atomic): Use call-with-nonlocal-exit-protect so
  that we don't mess up backtraces by catching all and then rethrowing.
  Should fix this more comprehensively somewhere, though.
2008-10-31 13:28:06 +01:00
Andy Wingo
46ccd0bbf9 make define-inline more usable from external modules
* module/system/il/inline.scm (define-inline): Use @ when accessing
  module vars so that other modules don't need to import all of our
  modules. However case-lambda is still needed.
2008-10-31 13:26:48 +01:00
Andy Wingo
fd4da4fae6 rework the scheme translator so it's extensible by external modules
* module/language/scheme/translate.scm (*translate-table*)
  (define-scheme-translator): Rework the translator to have the clauses
  defined separately via the define-scheme-translator macro, so that
  external modules can define their own translators. Should be no
  functional change in this commit, though.
2008-10-31 13:25:11 +01:00
Andy Wingo
4631414e29 compile goops submodules, goops.test now passes again
* libguile/goops.c (get_slot_value, set_slot_value): While keeping the
  inlined getter/setter dispatch for closures, allow the getters and
  setters to be any kind of procedure.

* oop/goops.scm (compute-getters-n-setters): Relax the checks on
  getter/setter procedures, so that if a getter is a procedure but not a
  closure, we don't try to poke its arity.

* oop/goops/Makefile.am (SOURCES): Compile all the goops submodules!

* oop/goops/old-define-method.scm: Removed, in an act of housekeeping.

* oop/goops/compile.scm:
* oop/goops/dispatch.scm: Break a circular module dependency by making
  sure that (oop goops) is loaded when we go to compile submodules.

* oop/goops/compile.scm (compile-method/memoizer)
  (compile-method/memoizer+next): Allow a procedure without source
  through. This can happen with getter and setter lambdas that were
  compiled, and in that case there is no next-method call anyway. Ideally
  we should be able to specify compile-method for accessor methods...
2008-10-31 11:35:47 +01:00
Andy Wingo
fd7ac322a5 fix chaining up from interpreted to compiled methods; allow compiled init-thunk
* libguile/goops.c (scm_sys_initialize_object): Don't assume that an init
  thunk is a closure; just go through scm_call_0 instead.

* oop/goops/compile.scm (make-make-next-method/memoizer): Allow for the
  case that the next method is compiled.
2008-10-31 10:14:49 +01:00
Ludovic Courtès
47b6e9bd8e Don't invoke `on_thread_exit ()' from a pthread key destructor.
The `on_thread_exit ()' function allocates memory via libgc.  When
called from the context of a pthread key detructor, the thread is
essentially "dead" already and `GC_lookup_thread ()' returns NULL,
which triggers an assertion in libgc's `thread_local_alloc.c'.  This
patch arranges so that `on_thread_exit ()' is called from a suitable
context.

* libguile/threads.c (on_thread_exit): Remove now invalid comment
  about access to libgc's TLS.
  (init_thread_key): Don't pass `on_thread_exit ()' to
  `scm_i_pthread_key_create ()'.
  (scm_leave_guile_cleanup): Invoke `do_thread_exit ()'.
  (really_launch): Invoke `pthread_exit ()'.
2008-10-31 00:27:20 +01:00
Andy Wingo
7d38f3d819 compile goops
The pending task is to make the accessors compiled too, and also to
compile compile.scm and dispatch.scm, and to integrate dispatch into the
VM.

* oop/Makefile.am (SOURCES): VM-ify the makefile, so we compile goops.scm
  by default.

* oop/goops.scm (load-toplevel): Load goops builtins when compiling too.
  (method): Fix a literal #<unspecified> in the generated procedure (for
  an empty body).
  (internal-add-method!): Cleverness when bootstrapping add-method!.
  Neat!
  (initialize for <generic>): Use the `method' macro so we get
  compilation support.

* oop/goops/dispatch.scm (cache-methods): Don't assume entries are pairs.
2008-10-31 00:07:04 +01:00
Andy Wingo
41a2772c5c compile occam-channel
* ice-9/Makefile.am (SOURCES): Compile the goops-using occam-channel.scm.
2008-10-30 15:53:11 +01:00
Andy Wingo
05b37c17ff fix up some assumptions that cmethods were lists
* libguile/eval.i.c (type_dispatch, apply_vm_cmethod)
  (apply_memoized_cmethod): Tweak the nastiness a bit more so as to deal
  with the '(no-method) empty entries. I would like to stop the search if
  the cdr isn't a pair, but currently with the inlined memoized bits, the
  cdr is a pair. The fix would be to make the memoizer return a procedure
  and not the already-inlined bits -- slightly slower but the vm will be
  faster anyway.

* libguile/objects.c (scm_mcache_lookup_cmethod): Same fixes here.

* oop/goops/dispatch.scm (cache-hashval, cache-try-hash!): Allow non-list
  cmethod tails.
2008-10-30 15:50:48 +01:00
Andy Wingo
5487977b1b runtime byte compilation of goops methods, whooooo
* ice-9/boot-9.scm (make-modules-in): Change to make sure that we are
  making modules in modules; that is, that a global binding of `compile'
  doesn't prevent a module from importing a submodule named `compile'.
  (resolve-module): Clean up a bit, and serialize the logic.

* libguile/objects.c (scm_mcache_lookup_cmethod, scm_apply_generic):
* libguile/eval.i.c (CEVAL): Now that cmethod entries can have a program
  as their tail instead of a memoized proc, we have to change the halting
  condition on the method cache search, in both places: the one that's
  inlined into eval.i.c and the one in objects.c. If the cmethod isn't a
  pair, apply it.

* libguile/goops.c (make): In the `make' procedure that's used before
  GOOPS is booted, bind #:formals, #:body, and #:compile-env on methods.

* oop/goops/compile.scm (compute-entry-with-cmethod): There was a
  terrible trick here that involved putting a dummy pair in the cache,
  then modifying it in place with the result of memoization. The note
  claimed that this was to cut recursion short, or something. I can't see
  how it could recurse, given that `methods' is changing each time. Also,
  the pair trick doesn't work with byte-compiled methods. So, remove it.
  (compile-method): Dispatch to the appropriate method compiler, based on
  whether the method was defined with the interpreter or with the
  compiler.
  (make-next-method): New function, generically computes a `next-method'
  procedure, though the caller has to supply the arguments.
  (compile-method/vm): Exciting method byte compiler!
  (make-make-next-method/memoizer, compile-method/memoizer): Add the
  /memoizer suffix, and move all this code to the bottom of the file.
2008-10-30 13:42:40 +01:00
Andy Wingo
3de80ed52f recompiling with compile environments, fluid languages, cleanups
* ice-9/boot-9.scm (compile-time-environment): Remove definition from
  boot-9 -- instead, autoload it and `compile' from (system base
  compile).

* libguile/objcodes.h:
* libguile/objcodes.c (scm_objcode_to_program): Add an optional argument,
  `external', the external list to set on the returned program.

* libguile/vm-i-system.c (externals): New instruction, returns the
  external list. Only used by (compile-time-environment).

* libguile/vm.c (scm_load_compiled_with_vm): Adapt to
  scm_objcode_to_program change.

* module/language/scheme/translate.scm (translate): Actually pay
  attention to the environment passed as an argument.
  (custom-transformer-table): Expand out (compile-time-environment) to
  something that can be passed to `compile'.

* module/system/base/compile.scm (*current-language*): Instead of
  hard-coding `scheme' in various places, use a current language fluid,
  initialized to `scheme'.
  (compile-file, load-source-file): Adapt to *current-language*.
  (load-source-file): Ada
  (scheme-eval): Removed, no one used this.
  (compiled-file-name): Don't hard-code "scm" and "go"; instead use the
  %load-extensions and %load-compiled-extensions.
  (cenv-module, cenv-ghil-env, cenv-externals): Some accessors for
  compile-time environments.
  (compile-time-environment): Here we define (compile-time-environment)
  to something that will return #f; the compiler however produces
  different code as noted above.
  (compile): New function, compiles an expression into a thunk, then runs
  the thunk to get the value. Useful for procedures. The optional second
  argument can be either a module or a compile-time-environment; in the
  latter case, we can recompile even with lexical bindings.
  (compile-in): If the env specifies a module, set that module for the
  duration of the compilation.

* module/system/base/syntax.scm (%compute-initargs): Fix a bug where the
  default value for a field would always replace a user-supplied value.
  Whoops.

* module/system/il/ghil.scm (ghil-env-dereify): New function, takes the
  result of ghil-env-reify and turns it back into a GHIL environment.

* scripts/compile (compile): Remove some of the tricky error handling, as
  the library procedures handle this for us.

* test-suite/tests/compiler.test: Add a test for the dynamic compilation
  bits.
2008-10-30 10:57:36 +01:00
Ludovic Courtès
979172b656 Document the impossibility to call the GC from within `on_thread_exit ()'.
* libguile/threads.c (on_thread_exit): Add `FIXME' comment.
2008-10-28 09:52:51 +01:00
Neil Jerram
9c646eee43 Fix stack calibration-related errors when running make distcheck.
* libguile/Makefile.am (stack-limit-calibration.scm): Use $(srcdir), to
  support building in a different directory.
  (MOSTLYCLEANFILES): Add stack-limit-calibration.scm.
2008-10-26 21:45:33 +00:00
Andy Wingo
21497600d2 add formals', body', and `compile-env' slots to <method>
* ice-9/boot-9.scm (compile-time-environment): Return #f instead of
  erroring under the interpreter, a bit more sane.

* libguile/goops.c (create_standard_classes):
* libguile/goops.h (scm_si_formals, scm_si_body, scm_si_compile_env):
* oop/goops.scm (method, initialize): Add `formals', `body', and
  `compile-env' slots to <method>.
2008-10-25 22:58:48 +02:00
Andy Wingo
ae9ce4b786 defmacroize (oop goops accessors), (oop goops save)
* oop/goops/accessors.scm (define-class-with-accessors)
  (define-class-with-accessors-keywords): Turn into defmacros.

* oop/goops/save.scm (readable, restore, write-component): Turn into
  defmacros.

Both of these changes are untested, unfortunately.
2008-10-25 22:58:48 +02:00
Andy Wingo
20bdc71054 add `compile-time-environment'
* ice-9/boot-9.scm (compile-time-environment): New function, with
  documentation. The trick is that the compiler recognizes calls to
  (compile-time-environment) and replaces it with a representation of the
  *available* lexicals. Note that this might not be all the lexicals;
  only the heap-allocated ones are returned.

* module/language/scheme/translate.scm (custom-transformer-table):
  Compile `compile-time-environment' to <ghil-reified-env>.

* module/system/il/compile.scm (codegen): Add <ghil-reified-env> clause,
  which calls ghil-env-reify.

* module/system/il/ghil.scm (ghil-env-reify): New procedure, returns a
  list of (NAME . EXTERNAL-INDEX).
  (<ghil>): Add <ghil-reified-env> object.
2008-10-25 22:58:48 +02:00
Andy Wingo
1086fabdc9 define-type no longer expects `|' subform
* module/system/base/syntax.scm (define-type): Rework to not require the
  `|', which confuses Emacs.

* module/system/il/ghil.scm (<ghil>):
* module/system/il/glil.scm (<glil>): Adapt to define-type changes.
2008-10-25 22:58:48 +02:00
Neil Jerram
7776113a28 Add measure-hwm.scm to the set of distribution files.
* libguile/Makefile.am (EXTRA_DIST): Add measure-hwm.scm.
2008-10-24 23:14:20 +01:00
Neil Jerram
f9d44e8476 Fix hang in srfi-18.test
* libguile/threads.h (held_mutex): New field.

	* libguile/threads.c (enqueue, remqueue, dequeue): Use critical
	section to protect access to the queue.
	(guilify_self_1): Initialize held_mutex field.
	(on_thread_exit): If held_mutex non-null, unlock it.
	(fat_mutex_unlock, fat_cond_free, scm_make_condition_variable,
	fat_cond_signal, fat_cond_broadcast): Delete now unnecessary uses
	of c->lock.
	(fat_mutex_unlock): Pass m->lock to block_self() instead of
	c->lock; move scm_i_pthread_mutex_unlock(m->lock) call from before
	block_self() to after.
	(scm_pthread_cond_wait, scm_pthread_cond_timedwait,
	scm_i_thread_sleep_for_gc): Set held_mutex before pthread call;
	reset it afterwards.

I was seeing a hang in srfi-18.test, when running make check in master,
in the "exception handler installation is thread-safe" test.  It wasn't
100% reproducible, so looked like a race.

The problem is that wait-condition-variable is not actually
atomic in the way that it is supposed to be.  It unlocks the mutex,
then starts waiting on the cond var.  So it is possible for another
thread to lock the same mutex, and signal the cond var, before the
wait-condition-variable thread starts waiting.

In order for wait-condition-variable to be atomic - e.g. in a race
where thread A holds (Scheme-level) mutex M, and calls
(wait-condition-variable C M), and thread B calls (begin (lock-mutex
M) (signal-condition-variable C)) - it needs to call pthread_cond_wait
with the same underlying mutex as is involved in the `lock-mutex'
call.  In terms of the threads.c code, this means that it has to use
M->lock, not C->lock.

block_self() used its mutex arg for two purposes: for protecting
access and changes to the wait queue, and for the pthread_cond_wait
call.  But it wouldn't work reliably to use M->lock to protect C's
wait queue, because in theory two threads can call
(wait-condition-variable C M1) and (wait-condition-variable C M2)
concurrently, with M1 and M2 different.  So we either have to pass
both C->lock and M->lock into block_self(), or use some other mutex to
protect the wait queue.  For this patch, I switched to using the
critical section mutex, because that is a global and so easily
available.  (If that turns out to be a problem for performance, we
could make each queue structure have its own mutex, but there's no
reason to believe yet that it is a problem, because the critical
section mutex isn't used much overall.)

So then we call block_self() with M->lock, and move where M->lock is
unlocked to after the block_self() call, instead of before.

That solves the first hang, but introduces a new one, when a SRFI-18
thread is terminated (`thread-terminate!') between being launched
(`make-thread') and started (`thread-start!').  The problem now is
that pthread_cond_wait is a cancellation point (see man
pthread_cancel), so the pthread_cond_wait call is one of the few
places where a thread-terminate! call can take effect.  If the thread
is cancelled at that point, M->lock ends up still being locked, and
then when do_thread_exit() tries to lock M->lock again, it hangs.

The fix for that is a new `held_mutex' field in scm_i_thread, which is
set to point to the mutex just before a pthread_cond_(timed)wait call,
and set to NULL again afterwards.  If on_thread_exit() finds that
held_mutex is non-NULL, it unlocks that mutex.

A detail is that checking and unlocking held_mutex must be done before
on_thread_exit() calls scm_i_ensure_signal_delivery_thread(), because
the innards of scm_i_ensure_signal_delivery_thread() can do another
pthread_cond_wait() call and so overwrite held_mutex.  But that's OK,
because it's fine for the mutex check and unlock to happen outside
Guile mode.

Lastly, C->lock is then not needed, so I've removed it.
2008-10-24 22:08:59 +01:00
Neil Jerram
d2a510879b Fix hang in srfi-18.test
* libguile/threads.h (held_mutex): New field.

	* libguile/threads.c (enqueue, remqueue, dequeue): Use critical
	section to protect access to the queue.
	(guilify_self_1): Initialize held_mutex field.
	(on_thread_exit): If held_mutex non-null, unlock it.
	(fat_mutex_unlock, fat_cond_free, scm_make_condition_variable,
	fat_cond_signal, fat_cond_broadcast): Delete now unnecessary uses
	of c->lock.
	(fat_mutex_unlock): Pass m->lock to block_self() instead of
	c->lock; move scm_i_pthread_mutex_unlock(m->lock) call from before
	block_self() to after.
	(scm_pthread_cond_wait, scm_pthread_cond_timedwait,
	scm_i_thread_sleep_for_gc): Set held_mutex before pthread call;
	reset it afterwards.

I was seeing a hang in srfi-18.test, when running make check in master,
in the "exception handler installation is thread-safe" test.  It wasn't
100% reproducible, so looked like a race.

The problem is that wait-condition-variable is not actually
atomic in the way that it is supposed to be.  It unlocks the mutex,
then starts waiting on the cond var.  So it is possible for another
thread to lock the same mutex, and signal the cond var, before the
wait-condition-variable thread starts waiting.

In order for wait-condition-variable to be atomic - e.g. in a race
where thread A holds (Scheme-level) mutex M, and calls
(wait-condition-variable C M), and thread B calls (begin (lock-mutex
M) (signal-condition-variable C)) - it needs to call pthread_cond_wait
with the same underlying mutex as is involved in the `lock-mutex'
call.  In terms of the threads.c code, this means that it has to use
M->lock, not C->lock.

block_self() used its mutex arg for two purposes: for protecting
access and changes to the wait queue, and for the pthread_cond_wait
call.  But it wouldn't work reliably to use M->lock to protect C's
wait queue, because in theory two threads can call
(wait-condition-variable C M1) and (wait-condition-variable C M2)
concurrently, with M1 and M2 different.  So we either have to pass
both C->lock and M->lock into block_self(), or use some other mutex to
protect the wait queue.  For this patch, I switched to using the
critical section mutex, because that is a global and so easily
available.  (If that turns out to be a problem for performance, we
could make each queue structure have its own mutex, but there's no
reason to believe yet that it is a problem, because the critical
section mutex isn't used much overall.)

So then we call block_self() with M->lock, and move where M->lock is
unlocked to after the block_self() call, instead of before.

That solves the first hang, but introduces a new one, when a SRFI-18
thread is terminated (`thread-terminate!') between being launched
(`make-thread') and started (`thread-start!').  The problem now is
that pthread_cond_wait is a cancellation point (see man
pthread_cancel), so the pthread_cond_wait call is one of the few
places where a thread-terminate! call can take effect.  If the thread
is cancelled at that point, M->lock ends up still being locked, and
then when do_thread_exit() tries to lock M->lock again, it hangs.

The fix for that is a new `held_mutex' field in scm_i_thread, which is
set to point to the mutex just before a pthread_cond_(timed)wait call,
and set to NULL again afterwards.  If on_thread_exit() finds that
held_mutex is non-NULL, it unlocks that mutex.

A detail is that checking and unlocking held_mutex must be done before
on_thread_exit() calls scm_i_ensure_signal_delivery_thread(), because
the innards of scm_i_ensure_signal_delivery_thread() can do another
pthread_cond_wait() call and so overwrite held_mutex.  But that's OK,
because it's fine for the mutex check and unlock to happen outside
Guile mode.

Lastly, C->lock is then not needed, so I've removed it.
2008-10-24 21:51:47 +01:00