Discussion:
[PATCH] catch exceptions in finalizers, remove dynamic resizing of finalizer vector
(too old to reply)
Felix
2012-06-19 09:35:36 UTC
Permalink
The attached patch adds exception handling around the invocation of
finalizers, which are shown as warnings (unless warnings are disabled)
but do not otherwise trigger errors (similar to the way errors in
separate threads are handled). I still experienced random crashes when
running the stress test in #866. What helped was removing the dynamic
resizing of the vector stored in ##sys#pending-finalizers. I could
not found a decent explanation for this but having a vector of fixed
size made the crashes go away. I assume there is some race-condition
between code that runs pending finalizers and code that creates new
ones ("set-finalizer!"). This means the number of finalizers is
limited to (currently) 4096. Code that produces many finalizers must
make sure they are triggered (and thus un-registered) fast enough.
Note that the "-:f" runtime option can be used to change the number of
available finalizers.


cheers,
felix
Jim Ursetto
2012-06-19 14:34:43 UTC
Permalink
Might I suggest that these nice thorough explanations be put in the
patch message itself (preceded by a blank line), so we always
have them at hand.
Post by Felix
The attached patch adds exception handling around the invocation of
finalizers, which are shown as warnings (unless warnings are disabled)
but do not otherwise trigger errors (similar to the way errors in
separate threads are handled). I still experienced random crashes when
running the stress test in #866. What helped was removing the dynamic
resizing of the vector stored in ##sys#pending-finalizers. I could
not found a decent explanation for this but having a vector of fixed
size made the crashes go away. I assume there is some race-condition
between code that runs pending finalizers and code that creates new
ones ("set-finalizer!"). This means the number of finalizers is
limited to (currently) 4096. Code that produces many finalizers must
make sure they are triggered (and thus un-registered) fast enough.
Note that the "-:f" runtime option can be used to change the number of
available finalizers.
cheers,
felix
From fcadbb82750d10d0c01178ffe603d9b5cfcf1731 Mon Sep 17 00:00:00 2001
Date: Thu, 14 Jun 2012 09:34:25 +0200
Subject: [PATCH] Catch exceptions in finalizers and added tests.
Resizing of the finalizer-table has been removed. There can be at most
4096 live finalizers (can be changed by using -:f).
---
chicken.h | 1 -
distribution/manifest | 1 +
library.scm | 49 ++++++++++++++++++++++++++++-----------
runtime.c | 14 -----------
scheduler.scm | 47 ++++++++++++++-----------------------
tests/finalizer-error-test.scm | 17 ++++++++++++++
tests/runtests.sh | 4 +-
7 files changed, 73 insertions(+), 60 deletions(-)
create mode 100644 tests/finalizer-error-test.scm
diff --git a/chicken.h b/chicken.h
index 837a51c..62dd1bc 100644
--- a/chicken.h
+++ b/chicken.h
@@ -1563,7 +1563,6 @@ C_fctexport void C_fcall C_paranoid_check_for_interrupt(void) C_regparm;
C_fctexport void C_zap_strings(C_word str);
C_fctexport void C_set_or_change_heap_size(C_word heap, int reintern);
C_fctexport void C_do_resize_stack(C_word stack);
-C_fctexport C_word C_resize_pending_finalizers(C_word size);
C_fctexport void C_initialize_lf(C_word *lf, int count);
C_fctexport void *C_register_lf(C_word *lf, int count);
C_fctexport void *C_register_lf2(C_word *lf, int count, C_PTABLE_ENTRY *ptable);
diff --git a/distribution/manifest b/distribution/manifest
index 6c02c34..02bc6ec 100644
--- a/distribution/manifest
+++ b/distribution/manifest
@@ -189,6 +189,7 @@ tests/functor-tests.scm
tests/square-functor.scm
tests/use-square-functor.scm
tests/pp-test.scm
+tests/finalizer-error-test.scm
tests/reverser/tags/1.0/reverser.meta
tests/reverser/tags/1.0/reverser.setup
tests/reverser/tags/1.0/reverser.scm
diff --git a/library.scm b/library.scm
index 030fad8..3387924 100644
--- a/library.scm
+++ b/library.scm
@@ -4574,18 +4574,13 @@ EOF
(define set-finalizer!
(lambda (x y)
(when (fx> (##sys#fudge 26) _max_pending_finalizers)
- (if (##core#inline "C_resize_pending_finalizers" (fx* 2 _max_pending_finalizers))
- (begin
- (set! ##sys#pending-finalizers (##sys#grow-vector ##sys#pending-finalizers
- (fx+ (fx* 2 _max_pending_finalizers) 1)
- (##core#undefined)))
- (when (##sys#fudge 13)
- (print "[debug] too many finalizers (" (##sys#fudge 26)
- "), resized max finalizers to " _max_pending_finalizers "...") ) )
- (begin
- (when (##sys#fudge 13)
- (print "[debug] too many finalizers (" (##sys#fudge 26) "), forcing ...") )
- (##sys#force-finalizers) ) ) )
+ (when (##sys#fudge 13)
+ (print "[debug] too many finalizers (" (##sys#fudge 26) "), forcing ...") )
+ (##sys#force-finalizers)
+ (when (fx> (##sys#fudge 26) _max_pending_finalizers)
+ (##sys#signal-hook
+ #:memory-error 'set-finalizer!
+ "maximal finalizer-count exceeded")))
(##sys#set-finalizer! x y) ) )
(define ##sys#run-pending-finalizers
@@ -4601,8 +4596,10 @@ EOF
(do ([i 0 (fx+ i 1)])
((fx>= i c))
(let ([i2 (fx+ 1 (fx* i 2))])
- ((##sys#slot ##sys#pending-finalizers (fx+ i2 1))
- (##sys#slot ##sys#pending-finalizers i2)) ) )
+ (handle-exceptions ex
+ (##sys#show-exception-warning ex "in finalizer" #f)
+ ((##sys#slot ##sys#pending-finalizers (fx+ i2 1))
+ (##sys#slot ##sys#pending-finalizers i2)) ) ))
(vector-fill! ##sys#pending-finalizers (##core#undefined))
(##sys#setislot ##sys#pending-finalizers 0 0)
(set! working #f) ) )
@@ -4741,6 +4738,30 @@ EOF
(writeargs (list ex) port) ] ) ) ) ) )
+;;; Show exception message and backtrace as warning
+;;; (used for threads and finalizers)
+
+(define ##sys#show-exception-warning
+ (let ((print-error-message print-error-message)
+ (display display)
+ (write-char write-char)
+ (print-call-chain print-call-chain)
+ (open-output-string open-output-string)
+ (get-output-string get-output-string) )
+ (lambda (exn cause #!optional (thread ##sys#current-thread))
+ (when ##sys#warnings-enabled
+ (let ((o (open-output-string)))
+ (display "Warning" o)
+ (when thread
+ (display " (" o)
+ (display thread o)
+ (write-char #\) o))
+ (display ": " o)
+ (display cause o)
+ (print-error-message exn ##sys#standard-error (get-output-string o))
+ (print-call-chain ##sys#standard-error 0 thread) ) ))))
+
+
(define (##sys#make-locative obj index weak? loc)
diff --git a/runtime.c b/runtime.c
index ced344b..bdaa335 100644
--- a/runtime.c
+++ b/runtime.c
@@ -1108,20 +1108,6 @@ void C_check_nursery_minimum(C_word words)
panic(C_text("nursery is too small - try higher setting using the `-:s' option"));
}
-C_word C_resize_pending_finalizers(C_word size) {
- int sz = C_num_to_int(size);
-
- FINALIZER_NODE **newmem =
- (FINALIZER_NODE **)C_realloc(pending_finalizer_indices, sz * sizeof(FINALIZER_NODE *));
-
- if (newmem == NULL)
- return C_SCHEME_FALSE;
-
- pending_finalizer_indices = newmem;
- C_max_pending_finalizers = sz;
- return C_SCHEME_TRUE;
-}
-
/* Parse runtime options from command-line: */
diff --git a/scheduler.scm b/scheduler.scm
index e3a96bc..d3a2620 100644
--- a/scheduler.scm
+++ b/scheduler.scm
@@ -309,35 +309,24 @@ EOF
(##sys#setislot t 4 #f)
(##sys#add-to-ready-queue t) )
-(define ##sys#default-exception-handler
- (let ([print-error-message print-error-message]
- [display display]
- [print-call-chain print-call-chain]
- [open-output-string open-output-string]
- [get-output-string get-output-string] )
- (lambda (arg)
- (let ([ct ##sys#current-thread])
- (dbg "exception: " ct " -> "
- (if (##sys#structure? arg 'condition) (##sys#slot arg 2) arg))
- (cond [(foreign-value "C_abort_on_thread_exceptions" bool)
- (let* ([pt ##sys#primordial-thread]
- [ptx (##sys#slot pt 1)] )
- (##sys#setslot
- pt 1
- (lambda ()
- (##sys#signal arg)
- (ptx) ) )
- (##sys#thread-unblock! pt) ) ]
- [##sys#warnings-enabled
- (let ([o (open-output-string)])
- (display "Warning (" o)
- (display ct o)
- (display ")" o)
- (print-error-message arg ##sys#standard-error (get-output-string o))
- (print-call-chain ##sys#standard-error 0 ct) ) ] )
- (##sys#setslot ct 7 arg)
- (##sys#thread-kill! ct 'terminated)
- (##sys#schedule) ) ) ) )
+(define (##sys#default-exception-handler arg)
+ (let ([ct ##sys#current-thread])
+ (dbg "exception: " ct " -> "
+ (if (##sys#structure? arg 'condition) (##sys#slot arg 2) arg))
+ (cond ((foreign-value "C_abort_on_thread_exceptions" bool)
+ (let* ([pt ##sys#primordial-thread]
+ [ptx (##sys#slot pt 1)] )
+ (##sys#setslot
+ pt 1
+ (lambda ()
+ (##sys#signal arg)
+ (ptx) ) )
+ (##sys#thread-unblock! pt) ) )
+ (else
+ (##sys#show-exception-warning arg "in thread" ct)))
+ (##sys#setslot ct 7 arg)
+ (##sys#thread-kill! ct 'terminated)
+ (##sys#schedule) ) )
diff --git a/tests/finalizer-error-test.scm b/tests/finalizer-error-test.scm
new file mode 100644
index 0000000..cf24da9
--- /dev/null
+++ b/tests/finalizer-error-test.scm
@@ -0,0 +1,17 @@
+;;;; finalizer-error-test.scm - by "megane"
+
+(define n 10000)
+
+(define (make-objects n)
+ (let loop [(i 0)]
+ (let [(o (make-vector 100))]
+ ;(print "making " i)
+ (set-finalizer! o (lambda (ob) (print* " " i)))
+ (if (< i n)
+ (loop (+ 1 i))))))
+
+(set-finalizer! (make-vector 100) (lambda (ob) (+ i 'a)))
+
+(make-objects n)
+
+(print "done")
diff --git a/tests/runtests.sh b/tests/runtests.sh
index 9f9f7ee..323c370 100755
--- a/tests/runtests.sh
+++ b/tests/runtests.sh
@@ -341,8 +341,8 @@ $compile symbolgc-tests.scm
echo "======================================== finalizer tests ..."
$interpret -s test-finalizers.scm
-
-echo "======================================== finalizer tests (2) ..."
+$compile finalizer-error-test.scm
+./a.out
$compile test-finalizers-2.scm
./a.out
--
1.6.0.4
_______________________________________________
Chicken-hackers mailing list
https://lists.nongnu.org/mailman/listinfo/chicken-hackers
Felix
2012-06-19 18:03:27 UTC
Permalink
From: Jim Ursetto <***@gmail.com>
Subject: Re: [Chicken-hackers] [PATCH] catch exceptions in finalizers, remove dynamic resizing of finalizer vector
Date: Tue, 19 Jun 2012 09:34:43 -0500
Post by Jim Ursetto
Might I suggest that these nice thorough explanations be put in the
patch message itself (preceded by a blank line), so we always
have them at hand.
Sure. Do you want one blank line or two? I can add some ASCII-art, too,
if you like that sort of thing. On the other hand: feel free to sign off
the patch and have a go yourself.

But yes, I will keep this in mind for the patch after the next.


cheers,
felix
Jim Ursetto
2012-06-19 18:14:25 UTC
Permalink
Hi Felix,
Post by Felix
Post by Jim Ursetto
Might I suggest that these nice thorough explanations be put in the
patch message itself (preceded by a blank line), so we always
have them at hand.
Sure. Do you want one blank line or two? I can add some ASCII-art, too,
if you like that sort of thing.
The blank line is a git convention so that it separates the summary
from the long description in git log --oneline. You can add an
extra blank line if you feel particularly saucy that day.
ASCII-art would be acceptable too, I miss the old banners.
Post by Felix
On the other hand: feel free to sign off
the patch and have a go yourself.
I thought it was ok to comment on patches without signing off.
I would have signed off if I felt qualified to review it.
Post by Felix
But yes, I will keep this in mind for the patch after the next.
Cool, thanks!

Jim
Felix
2012-06-19 18:22:39 UTC
Permalink
Post by Jim Ursetto
The blank line is a git convention so that it separates the summary
from the long description in git log --oneline. You can add an
extra blank line if you feel particularly saucy that day.
ASCII-art would be acceptable too, I miss the old banners.
Oh, man - you too? Ah, those were the days ...
Post by Jim Ursetto
Post by Felix
On the other hand: feel free to sign off
the patch and have a go yourself.
I thought it was ok to comment on patches without signing off.
Yes, of course. You did not really think this might be unwanted?


cheers,
felix
John Cowan
2012-06-19 18:48:21 UTC
Permalink
Post by Felix
Post by Jim Ursetto
ASCII-art would be acceptable too, I miss the old banners.
Oh, man - you too? Ah, those were the days ...
I always thought of the old Chicken banner as the Scheme version of the
CLISP banner:

i i i i i i i ooooo o ooooooo ooooo ooooo
I I I I I I I 8 8 8 8 8 o 8 8
I \ `+' / I 8 8 8 8 8 8
\ `-+-' / 8 8 8 ooooo 8oooo
`-__|__-' 8 8 8 8 8
| 8 o 8 8 o 8 8
------+------ ooooo 8oooooo ooo8ooo ooooo 8

Welcome to GNU CLISP 2.48 (2009-07-28) <http://clisp.cons.org/>

Copyright (c) Bruno Haible, Michael Stoll 1992, 1993
Copyright (c) Bruno Haible, Marcus Daniels 1994-1997
Copyright (c) Bruno Haible, Pierpaolo Bernardi, Sam Steingold 1998
Copyright (c) Bruno Haible, Sam Steingold 1999-2000
Copyright (c) Sam Steingold, Bruno Haible 2001-2009

Type :h and hit Enter for context help.
--
Work hard, John Cowan
play hard, ***@ccil.org
die young, http://www.ccil.org/~cowan
rot quickly.
Christian Kellermann
2012-06-20 07:45:17 UTC
Permalink
Post by John Cowan
Post by Felix
Post by Jim Ursetto
ASCII-art would be acceptable too, I miss the old banners.
Oh, man - you too? Ah, those were the days ...
I always thought of the old Chicken banner as the Scheme version of the
Does anyone still have that old one? Seems to have happened before my time...

_ ;-.-._
.-" "-. \. _{
/ \ / O )_
; | ; ,__(_<`
| / | \()
| /`\ ( | ;
\ \ | '-..-'; |\
'.;| ,_ _.= \ /`|
\ '. '-' |
\ '=. /
'. / .'
\ .'---';`
jgs | / `. |
_|| `\\
` -.'-- .-'_'--.
`" `--

(found via http://www.retrojunkie.com/asciiart/animals/chickens.htm)

--
9 out of 10 voices in my head say, that I am crazy,
one is humming.
John Cowan
2012-06-19 14:38:43 UTC
Permalink
This means the number of finalizers is limited to (currently) 4096. Code
that produces many finalizers must make sure they are triggered (and
thus un-registered) fast enough.
Ick, ick, ick. This is a horrible solution. The main use of finalizers
that I've ever had is when dealing with objects allocated by C libraries;
a pointer object wrapped in a finalizer insures that when the foreign
object is no longer interesting to the Scheme side, it is properly freed,
which may or may not involve calling free().

Having a fixed low limit on the number of finalizers means that this
sort of design is not practical: you wind up being better off with
larger-grain foreign objects and more C code to access their insides.
That's un-Schemey.
Note that the "-:f" runtime option can be used to change the number
of available finalizers.
This helps a little, but not much, because it's basically guesswork what
to set the option to, and it may vary greatly depending on the input,
leading to programs that work fine and then suddenly crash.

Why does there have to a be a persistent array of all finalizers anyway?
The garbage collector itself can find them during the mark phase.
--
John Cowan ***@ccil.org http://ccil.org/~cowan
This great college [Trinity], of this ancient university [Cambridge],
has seen some strange sights. It has seen Wordsworth drunk and Porson
sober. And here am I, a better poet than Porson, and a better scholar
than Wordsworth, somewhere betwixt and between. --A.E. Housman
Peter Bex
2012-06-19 16:41:17 UTC
Permalink
Post by John Cowan
This means the number of finalizers is limited to (currently) 4096. Code
that produces many finalizers must make sure they are triggered (and
thus un-registered) fast enough.
Ick, ick, ick. This is a horrible solution. The main use of finalizers
that I've ever had is when dealing with objects allocated by C libraries;
a pointer object wrapped in a finalizer insures that when the foreign
object is no longer interesting to the Scheme side, it is properly freed,
which may or may not involve calling free().
I completely agree here. This patch will make things worse.
Instead of hurrying for a release and making incomplete/incorrect patches
I think it's better to fix the bugs we have first.
Post by John Cowan
Why does there have to a be a persistent array of all finalizers anyway?
The garbage collector itself can find them during the mark phase.
The GC needs to be able to associate an object with its finalizers, so
there either needs to be a global list or some extra slot added to
each object. The latter takes up more memory when you don't have a lot
of finalizers (and finalizers are slow, so it's best not to generate too
many of them).

What I don't quite understand is all this talk about threads. The test
program doesn't even create any extra threads, so how could there be any
race conditions? There shouldn't *be* any concurrency in this case,
should there?

Cheers,
Peter
--
http://sjamaan.ath.cx
--
"The process of preparing programs for a digital computer
is especially attractive, not only because it can be economically
and scientifically rewarding, but also because it can be an aesthetic
experience much like composing poetry or music."
-- Donald Knuth
Felix
2012-06-19 18:19:08 UTC
Permalink
Post by Peter Bex
Post by John Cowan
Ick, ick, ick. This is a horrible solution. The main use of finalizers
that I've ever had is when dealing with objects allocated by C libraries;
a pointer object wrapped in a finalizer insures that when the foreign
object is no longer interesting to the Scheme side, it is properly freed,
which may or may not involve calling free().
I completely agree here. This patch will make things worse.
Instead of hurrying for a release and making incomplete/incorrect patches
I think it's better to fix the bugs we have first.
Nobody is hurrying. The patch is also not incorrect - it is a
simplification and removes a potential problem.
Post by Peter Bex
Post by John Cowan
Why does there have to a be a persistent array of all finalizers anyway?
The garbage collector itself can find them during the mark phase.
The GC needs to be able to associate an object with its finalizers, so
there either needs to be a global list or some extra slot added to
each object. The latter takes up more memory when you don't have a lot
of finalizers (and finalizers are slow, so it's best not to generate too
many of them).
Exactly. Finalizers are junk. Finalizers are the last resort. Creating
excessively many of them is a design error.

(That was just for the record)
Post by Peter Bex
What I don't quite understand is all this talk about threads. The test
program doesn't even create any extra threads, so how could there be any
race conditions? There shouldn't *be* any concurrency in this case,
should there?
There is concurrency between the garbage collector (that can kick in
at *any* moment) and library code. The GC has a linked list of
finalizers, allocated dynamically and theoretically unlimited in size
(and also nicely slowing down every major GC when you have lots of
them). But once finalizers are "triggered", they have to be stashed
somewhere. That somewhere is the "pending finalizers vector"
(##sys#pending-finalizers, IIRC) and that currently grows on demand.
I _assume_ (I don't know for sure), that there is some sort of problem
in the situation when ##sys#pending-finalizers must grow, and at the
same time is used by GC runtime system code to hold triggered
finalizers. It's all terribly complicated. And it's an absolute PAIN
to debug. We can probably figure out some funky clever solution for
this, something we can be really proud of. On the other hand, we can
just try to simplify things a bit, have a good old decent fixed size
buffer (out fathers weren't ashamed of them good old decent fixed size
buffers, right?), and tell those that put finalizers onto everything
to grow up.


cheers,
felix
Felix
2012-06-19 18:44:47 UTC
Permalink
Post by Felix
Post by Peter Bex
I completely agree here. This patch will make things worse.
Instead of hurrying for a release and making incomplete/incorrect patches
I think it's better to fix the bugs we have first.
Nobody is hurrying. The patch is also not incorrect - it is a
simplification and removes a potential problem.
That doesn't mean it can't wait. We live with this bug long enough, so
it can be postponed to a later release, if someone finds a better way
to handle this that is not increasing the ickiness and complexity of
the whole subject of finalizer implementation in an unduely way.


cheers,
felix
Peter Bex
2012-06-19 18:48:19 UTC
Permalink
Post by Felix
There is concurrency between the garbage collector (that can kick in
at *any* moment) and library code. The GC has a linked list of
finalizers, allocated dynamically and theoretically unlimited in size
(and also nicely slowing down every major GC when you have lots of
them). But once finalizers are "triggered", they have to be stashed
somewhere. That somewhere is the "pending finalizers vector"
(##sys#pending-finalizers, IIRC) and that currently grows on demand.
I _assume_ (I don't know for sure), that there is some sort of problem
in the situation when ##sys#pending-finalizers must grow, and at the
same time is used by GC runtime system code to hold triggered
finalizers.
Right now it looks to me like there's an optimization that's messing
things up. Try compiling this file with -O0 and then with -O1:

(let lp ((x (list 1 2 3)))
(set-finalizer! x (lambda _ #f))
(lp (list 1 2 3)))

With -O1 or higher this will raise a suspicious looking error:

Error: call of non-procedure: #<unspecified>
I also see this with 4.7.0, even when compiled with -O0. With -O1 on
4.7.0, it shows this error and _then_ craps out with an OOM error.

The version of Megane's test in your patch also simply succeeds when
compiled with -O0. I don't know if that's a coincidence, but that seems
a bit unlikely to me right now. I'll try digging deeper, but this will
take time.
Post by Felix
It's all terribly complicated. And it's an absolute PAIN
to debug. We can probably figure out some funky clever solution for
this, something we can be really proud of. On the other hand, we can
just try to simplify things a bit, have a good old decent fixed size
buffer (out fathers weren't ashamed of them good old decent fixed size
buffers, right?), and tell those that put finalizers onto everything
to grow up.
Some things can currently only be handled cleanly with finalizers
if you don't want terribly imperative code that will fail whenever
you accidentally forget to clean up a resource. I don't think we
should be forcing people to come up with workarounds to obviously
good solutions.

Cheers,
Peter
--
http://sjamaan.ath.cx
--
"The process of preparing programs for a digital computer
is especially attractive, not only because it can be economically
and scientifically rewarding, but also because it can be an aesthetic
experience much like composing poetry or music."
-- Donald Knuth
Jörg F. Wittenberger
2012-06-19 20:28:25 UTC
Permalink
Post by Peter Bex
Right now it looks to me like there's an optimization that's messing
(let lp ((x (list 1 2 3)))
(set-finalizer! x (lambda _ #f))
(lp (list 1 2 3)))
Error: call of non-procedure: #<unspecified>
I also see this with 4.7.0, even when compiled with -O0. With -O1 on
4.7.0, it shows this error and _then_ craps out with an OOM error.
This would kinda explain, why my "eat all memory" issue is so
apparent on the recent mint install I did and rare on older systems.

This system has gcc (Ubuntu/Linaro 4.6.1-9ubuntu3) 4.6.1
it is the first one to complain about chicken code for me:


atomic.c: In function 'f_5446': atomic.c:5062:4: warning: array subscript
is above array bounds [-Warray-bounds] atomic.c: In function 'f_3797':
atomic.c:1534:4: warning: array subscript is above array bounds
[-Warray-bounds] atomic.c: In function 'f_3760': atomic.c:7751:4: warning:
array subscript is above array bounds [-Warray-bounds] atomic.c: In
function 'f_3733': atomic.c:8312:4: warning: array subscript is above array
bounds [-Warray-bounds] atomic.c: In function 'f_3791':


Note that I'm seen the problem on code, which is compiled with -scrutinize
and gcc -O2.

This *might* hint somewhere…? Anybody running Clang?

/Jerry
.....
Felix
2012-06-23 11:52:36 UTC
Permalink
Post by Peter Bex
Right now it looks to me like there's an optimization that's messing
(let lp ((x (list 1 2 3)))
(set-finalizer! x (lambda _ #f))
(lp (list 1 2 3)))
Error: call of non-procedure: #<unspecified>
I also see this with 4.7.0, even when compiled with -O0. With -O1 on
4.7.0, it shows this error and _then_ craps out with an OOM error.
The version of Megane's test in your patch also simply succeeds when
compiled with -O0. I don't know if that's a coincidence, but that seems
a bit unlikely to me right now. I'll try digging deeper, but this will
take time.
Compiling without optimizations means more CPS calls, which means more
nursery allocation, which means more minor GCs, which means more major
GCs, which means faster triggering of finalizer which means less
pressure to resize ##sys#pending-finalizers. Everything that allocates
influences GC and thugs finalization. Normal execution of code
allocates activation frames.
Post by Peter Bex
Some things can currently only be handled cleanly with finalizers
if you don't want terribly imperative code that will fail whenever
you accidentally forget to clean up a resource. I don't think we
should be forcing people to come up with workarounds to obviously
good solutions.
I don't see how limiting the available number of finalizers "forces
people to come up with workarounds". We are talking about excessively
creating finalizers. In fact, the only code I've seen that actually
causes the problems are contrived finalizer-creation loops. I'm not
against the use of finalizers and I don't understand how you come to
that conclusion. They are a workaround that is, unfortunately,
sometimes necessary.


cheers,
felix
Jörg F. Wittenberger
2012-06-19 20:22:25 UTC
Permalink
Post by Felix
Post by Peter Bex
each object. The latter takes up more memory when you don't have a lot
of finalizers (and finalizers are slow, so it's best not to generate too
many of them).
Exactly. Finalizers are junk. Finalizers are the last resort. Creating
excessively many of them is a design error.
(That was just for the record)
Anecdotal evidence: I already began to convert my code, killing
finalizers the other day.

If I only knew a recipe to get around them, I'd vote to not have them
at all.

/Jerry
.......
John Cowan
2012-06-19 19:12:30 UTC
Permalink
Post by Peter Bex
The GC needs to be able to associate an object with its finalizers, so
there either needs to be a global list or some extra slot added to
each object. The latter takes up more memory when you don't have a lot
of finalizers (and finalizers are slow, so it's best not to generate too
many of them).
How about if finalizers are only attached to pointers? That seems to
be the principal use case, and adding a slot to a pointer (maybe only a
"finalizable tagged pointer" wouldn't be so expensive.
Post by Peter Bex
What I don't quite understand is all this talk about threads. The test
program doesn't even create any extra threads, so how could there be any
race conditions? There shouldn't *be* any concurrency in this case,
should there?
Even without the scheduler, the collector and the mutator are effectivevly
separate threads, because garbage collection happens unpredictably from
the point of view of the primordial thread.
--
A: "Spiro conjectures Ex-Lax." John Cowan
Q: "What does Pat Nixon frost her cakes with?" ***@ccil.org
--"Jeopardy" for generative semanticists http://www.ccil.org/~cowan
Alaric Snell-Pym
2012-06-19 20:38:40 UTC
Permalink
Post by John Cowan
Post by Peter Bex
The GC needs to be able to associate an object with its finalizers, so
there either needs to be a global list or some extra slot added to
each object. The latter takes up more memory when you don't have a lot
of finalizers (and finalizers are slow, so it's best not to generate too
many of them).
How about if finalizers are only attached to pointers? That seems to
be the principal use case, and adding a slot to a pointer (maybe only a
"finalizable tagged pointer" wouldn't be so expensive.
It would also be quite easy to attach such "finalizing objects" to
records, vectors, and the like (ensuring that that is the ONLY
reference) to get a notification of the parent object being GCed, too.

Ugh, that's not very clear.

What I'm saying is, you could create a finalizable tagged pointer that
doesn't actually point to anything, and only exists to be placed in an
otherwise unused slot of a record (for instance), such that when the
record is no longer reachable and gets GCed, the finalizable tagged
pointer is then also unreachable and gets finalized, acting as a proxy
for the otherwise-unfinalizable object. Perhaps the pointer could be
left pointing to whatever state might be needed to finalize the real
object, as the real object would be unreachable by the time the
finalizer was triggered.

ABS

--
Alaric Snell-Pym
http://www.snell-pym.org.uk/alaric/
Felix
2012-06-23 11:55:51 UTC
Permalink
From: John Cowan <***@mercury.ccil.org>
Subject: Re: [Chicken-hackers] [PATCH] catch exceptions in finalizers, remove dynamic resizing of finalizer vector
Date: Tue, 19 Jun 2012 15:12:30 -0400
Post by John Cowan
Post by Peter Bex
The GC needs to be able to associate an object with its finalizers, so
there either needs to be a global list or some extra slot added to
each object. The latter takes up more memory when you don't have a lot
of finalizers (and finalizers are slow, so it's best not to generate too
many of them).
How about if finalizers are only attached to pointers? That seems to
be the principal use case, and adding a slot to a pointer (maybe only a
"finalizable tagged pointer" wouldn't be so expensive.
Allowing finalization only for pointers would be an unecessary
restriction. Adding an extra slot to arbitrary objects is more
difficult than it sounds and has implications to just about
everything.


cheers,
felix
Jörg F. Wittenberger
2012-06-19 19:52:32 UTC
Permalink
Post by Peter Bex
What I don't quite understand is all this talk about threads. The test
program doesn't even create any extra threads, so how could there be any
race conditions? There shouldn't *be* any concurrency in this case,
should there?
Isn't this more of a text-book-example of an issue?

By definition a finalizer is a unit of execution to be scheduled
to run when a certain condition is met. Letting alone discussions
which data structure is called a thread: this requires some,
possibly minimal scheduling. Hence worry about concurrency.

BTW: there are many more such issues. E.g. "delay". The promise
is supposed to be computed just once. Try:

(use srfi-18)

(define foo (let ((called 0)) (delay (begin (set! called (+ called 1))
(thread-sleep! 10) called))))

(do ((i 0 (+ i 1))) ((= i 5)) (thread-start! (lambda () (force foo))))

(force foo)


To summarize: I subscribe to Felix view that it would be great to
have the core system thread free. However I'm honestly unsure
that this is at all possible, given that we have threads masquerading
under the name of promises and finalizers, which inherently require
some scheduling.

However I'd welcome to expel the scheduler from the base
system and instead have some simpler API, where I can register
a continuation to wait for events from files, time, signals
other threads and other resources. The current scheduler
as well as fictionally once based on zmq as suggested by <whom>
on the list the other day should have an easy time to hook
into this API. --- Quite a project IMHO.

Best Regards

/Jerry
.........
Felix
2012-06-23 11:58:15 UTC
Permalink
Post by Jörg F. Wittenberger
However I'd welcome to expel the scheduler from the base
system and instead have some simpler API, where I can register
a continuation to wait for events from files, time, signals
other threads and other resources. The current scheduler
as well as fictionally once based on zmq as suggested by <whom>
on the list the other day should have an easy time to hook
into this API. --- Quite a project IMHO.
Scheme-to-C had the interesting concept of "system tasks" where user
code could register period tasks that are called whenever the system
is idle and no input is available.


cheers,
felix

Felix
2012-06-19 18:06:31 UTC
Permalink
Post by John Cowan
This means the number of finalizers is limited to (currently) 4096. Code
that produces many finalizers must make sure they are triggered (and
thus un-registered) fast enough.
Ick, ick, ick. This is a horrible solution. The main use of finalizers
that I've ever had is when dealing with objects allocated by C libraries;
a pointer object wrapped in a finalizer insures that when the foreign
object is no longer interesting to the Scheme side, it is properly freed,
which may or may not involve calling free().
It probably isn't even a solution. I don't disagree with you here.
Post by John Cowan
Having a fixed low limit on the number of finalizers means that this
sort of design is not practical: you wind up being better off with
larger-grain foreign objects and more C code to access their insides.
That's un-Schemey.
It probably is.
Post by John Cowan
Note that the "-:f" runtime option can be used to change the number
of available finalizers.
This helps a little, but not much, because it's basically guesswork what
to set the option to, and it may vary greatly depending on the input,
leading to programs that work fine and then suddenly crash.
Yep. Just like running out of memory. There always will be some hard
limits.
Post by John Cowan
Why does there have to a be a persistent array of all finalizers anyway?
The garbage collector itself can find them during the mark phase.
It is much more complicated than that (see my reply to Peter).


cheers,
felix
Jörg F. Wittenberger
2012-06-19 18:51:00 UTC
Permalink
Post by Felix
The attached patch adds exception handling around the invocation of
finalizers, which are shown as warnings (unless warnings are disabled)
but do not otherwise trigger errors (similar to the way errors in
separate threads are handled). I still experienced random crashes when
running the stress test in #866. What helped was removing the dynamic
This maybe related, but maybe not at all.

To me it looks as a repeated occurrence of this problem:
http://lists.nongnu.org/archive/html/chicken-users/2008-09/msg00025.html

However my wild guess would be that the static array for finalizers
is a red herring.

It ran quite comfortable once finalizers where wraped by an exception
handler.

However I too see random crashes all day.

To me it looks more like a gc problem (wild guess work again).

I've been able to produce *some* binaries, usually just "minor" code
changes - that is minor wrt. Scheme - which would crash sooner or
later or exhibit another problem:

The same binary would start up different. As early as during the
initialization of all modules it select one of two modi: (A): run
normal for minutes or hour until switch to (B) and (B): eat up
at least 512kB mutation stack, slow down to a grind and repeat
resizing the mutation stack. (at this point my session freezes;
I never managed to be fast enough to stop and attach to the
process)


My most recent update to chicken trunk made this come up so frequetly
that I started these days to experiment with a workaround.

I introduced a mutation_stack_limit and forced a gc whenever
the mutation_stack has more than a constant (currently I'm using
1024) entries.

This made sure it never enters mode (B). At the cost of:

# define C_stack_probe(p) (C_stress && ((C_word *)(p) < C_stack_limit) &&
!C_mutation_limit)

you see: binary incompatible, runtime.c exports yet another C_word*
and each C_stack_check involves yet another memory access.

(((I've been wondering: this C_mutation_limit is actually a boolean.
Maybe I could fold it into the LSB the the C_stack_limit to avoid
the extra memory access.)))

This cost is so far bearable, given that I can continue to run
my older code as it was.

Coming back to "maybe related": It also went to the benefit to reduce
the felt frequency of those nasty segfaults…

…if only I could say, that is killed them. It did not.

That's what makes me believe the static array might be read herring.

Hope this helps.

/Jerry
.......
Christian Kellermann
2012-06-20 07:54:00 UTC
Permalink
Hi Felix!
Post by Felix
The attached patch adds exception handling around the invocation of
finalizers, which are shown as warnings (unless warnings are disabled)
but do not otherwise trigger errors (similar to the way errors in
separate threads are handled). I still experienced random crashes when
running the stress test in #866. What helped was removing the dynamic
resizing of the vector stored in ##sys#pending-finalizers. I could
not found a decent explanation for this but having a vector of fixed
size made the crashes go away. I assume there is some race-condition
between code that runs pending finalizers and code that creates new
ones ("set-finalizer!"). This means the number of finalizers is
limited to (currently) 4096. Code that produces many finalizers must
make sure they are triggered (and thus un-registered) fast enough.
Note that the "-:f" runtime option can be used to change the number of
available finalizers.
Thank you for this patch! I don't want to add to the discussion of how
to do this Right(tm), since this probably just means making design
decisions according to design goals and people arguing whether their
design goal is better than the other's...

What I want to do instead is to propose the split of the two things
the patch does into two separate patches. The fix for #866 is a good
one and I think that should go in. Then we can quarrel about the way
finalizers should be handled until either everyone gives up or maybe
we think of something else and provide a "better" patch based on a
common understanding of the problem.

Also I do think that it will not hurt the commit log when your
explanation is part of it. Sometimes I think of it when reviewing and
add it, most of the time its buried in the archives. I also know that
sometimes the time difference between making the patch and sending it
off to the list with the explanation may improve the latter ;)

So writing correct software is hard, drinks anyone?

Kind regards,

Christian

--
9 out of 10 voices in my head say, that I am crazy,
one is humming.
Loading...