headers
Thomas Christian Chust | 27 Jul 12:42

Suggested patch for Open Dylan's C runtime

Hello,

the current 1.0beta4 version of Open Dylan doesn't support finalizers
for objects in programs built with the C runtime that uses the
Boehm-Demers-Weiser garbage collector.

However, I think it is pretty easy to add that feature. I have tested
the attached patch for opendylan/sources/dfmc/c-run-time/run-time.{h,c}
on my PowerPC MacOS X installation of Open Dylan and it seems to work
just fine.

cu,
Thomas


--- _run-time.h	2007-07-26 20:34:57.000000000 +0200
+++ run-time.h	2007-07-26 20:44:01.000000000 +0200
@@ -1044,8 +1044,8 @@
 #define primitive_gc_state() (I(0)) /* !@#$ DUMMY DEFN */
 #define primitive_pin_object(x) (x)
 extern void primitive_unpin_object(D);
-#define primitive_mps_finalize(x) { }
-#define primitive_mps_finalization_queue_first() ((D)0)
+extern void primitive_mps_finalize(D);
+extern void* primitive_mps_finalization_queue_first();
 #define primitive_mps_park()
 #define primitive_mps_clamp()
 #define primitive_mps_release()
--- _run-time.c	2007-07-26 20:34:46.000000000 +0200
(Continue reading)

Permalink | Reply |
headers
Hannes Mehnert | 29 Jul 21:37

Re: Suggested patch for Open Dylan's C runtime

Hi Thomas,

I just commited that patch to the subversion repository. Thanks a lot.

Hannes

Thomas Christian Chust wrote:
> the current 1.0beta4 version of Open Dylan doesn't support finalizers
> for objects in programs built with the C runtime that uses the
> Boehm-Demers-Weiser garbage collector.
> 
> However, I think it is pretty easy to add that feature. I have tested
> the attached patch for opendylan/sources/dfmc/c-run-time/run-time.{h,c}
> on my PowerPC MacOS X installation of Open Dylan and it seems to work
> just fine.
--

-- 
Gd-hackers mailing list
Gd-hackers <at> gwydiondylan.org
https://www.opendylan.org/mailman/listinfo/gd-hackers
Permalink | Reply |
headers
Bruce Hoult | 30 Jul 00:17

Re: Suggested patch for Open Dylan's C runtime

On 7/30/07, Hannes Mehnert <hannes <at> mehnert.org> wrote:
> I just commited that patch to the subversion repository. Thanks a lot.

This patch appears to put finalized objects onto a queue for later
running, rather than running them from within the GC. Good!

BUT .. it does a GC_NEW() from within the finalizer, which is called
from within the GC.  Therefore the GC is entered recursively.  This is
very very bad and will bite you one day and you won't know why.
--

-- 
Gd-hackers mailing list
Gd-hackers <at> gwydiondylan.org
https://www.opendylan.org/mailman/listinfo/gd-hackers
Permalink | Reply |
headers
Hannes Mehnert | 30 Jul 13:58

Re: Suggested patch for Open Dylan's C runtime

Hi Bruce,

Bruce Hoult wrote:
> On 7/30/07, Hannes Mehnert <hannes <at> mehnert.org> wrote:
>> I just commited that patch to the subversion repository. Thanks a lot.
> 
> This patch appears to put finalized objects onto a queue for later
> running, rather than running them from within the GC. Good!
> 
> BUT .. it does a GC_NEW() from within the finalizer, which is called
> from within the GC.  Therefore the GC is entered recursively.  This is
> very very bad and will bite you one day and you won't know why.

Oh, that's bad. Have you got a patch around which prevents calling
GC_NEW within the finalizer? ;) I just added your comment to the
specific part of the code.

Hannes
--

-- 
Gd-hackers mailing list
Gd-hackers <at> gwydiondylan.org
https://www.opendylan.org/mailman/listinfo/gd-hackers
Permalink | Reply |
headers
Thomas Christian Chust | 30 Jul 14:48

Re: Suggested patch for Open Dylan's C runtime

Bruce Hoult wrote:

> On 7/30/07, Hannes Mehnert <hannes <at> mehnert.org> wrote:
>
>> I just commited that patch to the subversion repository. Thanks a lot.
> 
> This patch appears to put finalized objects onto a queue for later
> running, rather than running them from within the GC. Good!
> 
> BUT .. it does a GC_NEW() from within the finalizer, which is called
> from within the GC.  Therefore the GC is entered recursively.  This is
> very very bad and will bite you one day and you won't know why.

Hello,

I must admit that I don't know the Boehm garbage collector's internals
well enough to decide for sure whether allocation from a finalizer is
safe or not. In case the allocation functions hold some mutex locks this
could indeed break horribly.

But anyway, I think this is about the only way to force the Boehm
garbage collector into an approximately correct behaviour for
finalization queues: If I don't put the object pointer into a queue
allocated with the garbage collector, it gets discarded immediately
after the finalizer has run. Thus I must first make the pointer live
again by putting it into a block of memory allocated with the collector.
Once the queue has been drained, the queue memory itself and the objects
referenced from it will be eligible for collection again, unless the
Dylan finalizers resurrect some objects once more.
(Continue reading)

Permalink | Reply |
headers
Bruce Hoult | 30 Jul 15:30

Re: Suggested patch for Open Dylan's C runtime

On 7/31/07, Thomas Christian Chust <chust <at> web.de> wrote:
> But anyway, I think this is about the only way to force the Boehm
> garbage collector into an approximately correct behaviour for
> finalization queues: If I don't put the object pointer into a queue
> allocated with the garbage collector, it gets discarded immediately
> after the finalizer has run.

No, the existence of the finalizer has already saved it from that GC.
The space will only be reused if the object is not pointed to by
anything on the *next* GC.

> The only acceptable alternative I can think of that avoids allocation in
> the C-level finalizer would be to preallocate an array list to use as
> the finalization queue. But that would pose an artificial size limit on
> the finalization queue.

You could allocate the list link for the object at the point that you
register the finalizer for it (possibly as part of the object itself
if you have control over that by, for example, allocating an extra 4
bytes and lying to the rest of Dylan about where the object starts).

In fact if you want to guaranteed that finalizers will be run before
program exit then you have to do that anyway.  Then you'll have the
actual finalizer remove the object from the program exit cleanup list.

You can also use the facility in Boehm for asynchronous finalization
using the  GC_invoke_finalizers() function and either setting your own
function into the GC_finalizer_notifier variable or else polling
GC_should_invoke_finalizers().  So you can for example set
GC_finalizer_notifier to point to a function that clears a semaphore
(Continue reading)

Permalink | Reply |
headers
Thomas Christian Chust | 30 Jul 16:36

Re: Suggested patch for Open Dylan's C runtime

Bruce Hoult wrote:

> [...]
> You could allocate the list link for the object at the point that you
> register the finalizer for it (possibly as part of the object itself
> if you have control over that by, for example, allocating an extra 4
> bytes and lying to the rest of Dylan about where the object starts).

Hello,

I've attached a new patch replacing the other one which implements the
finalization queue as a dynamically sized array with resizing operations
taking place only as necessary in primitive_mps_finalize and
primitive_mps_finalization_queue_first. It seems to work just as well as
the first patch on my machine and I hope it is safer :-)

> In fact if you want to guaranteed that finalizers will be run before
> program exit then you have to do that anyway.  Then you'll have the
> actual finalizer remove the object from the program exit cleanup list.

The way finalization is implemented in Open Dylan, finalizers are not
guaranteed to be run on program exit, I think.

> You can also use the facility in Boehm for asynchronous finalization
> using the  GC_invoke_finalizers() function and either setting your own
> function into the GC_finalizer_notifier variable or else polling
> GC_should_invoke_finalizers().  So you can for example set
> GC_finalizer_notifier to point to a function that clears a semaphore
> that releases a thread that calls GC_invoke_finalizers().  No need to
> keep track of anything else yourself.  The GC won't delete the objects
(Continue reading)

Permalink | Reply |
headers
Thomas Christian Chust | 31 Jul 00:06

Re: Suggested patch for Open Dylan's C runtime

Thomas Christian Chust wrote:
> Bruce Hoult wrote:
> 
>> [...]
>> You could allocate the list link for the object at the point that you
>> register the finalizer for it (possibly as part of the object itself
>> if you have control over that by, for example, allocating an extra 4
>> bytes and lying to the rest of Dylan about where the object starts).
> 
> Hello,
> 
> I've attached a new patch replacing the other one which implements the
> finalization queue as a dynamically sized array with resizing operations
> taking place only as necessary in primitive_mps_finalize and
> primitive_mps_finalization_queue_first. It seems to work just as well as
> the first patch on my machine and I hope it is safer :-)
> [...]

Hello,

I guess I shouldn't hack such stuff together when in a hurry. A much
easier solution which also avoid allocation inside the finalization
procedure is possible by simply using the data argument to that
procedure to supply a preallocated cons cell.

So here is another patch, replacing the earlier ones, and I apologize
for not thinking before coding %-]

cu,
Thomas
(Continue reading)

Permalink | Reply |
headers
Bruce Hoult | 31 Jul 00:23

Re: Suggested patch for Open Dylan's C runtime

On 7/31/07, Thomas Christian Chust <chust <at> web.de> wrote:
> Thomas Christian Chust wrote:
> > Bruce Hoult wrote:
> >> You could allocate the list link for the object at the point that you
> >> register the finalizer for it
> >
> I guess I shouldn't hack such stuff together when in a hurry. A much
> easier solution which also avoid allocation inside the finalization
> procedure is possible by simply using the data argument to that
> procedure to supply a preallocated cons cell.

Right. :-)  Looks good provided that link object won't get GC'd in the meantime.
--

-- 
Gd-hackers mailing list
Gd-hackers <at> gwydiondylan.org
https://www.opendylan.org/mailman/listinfo/gd-hackers
Permalink | Reply |
headers
Thomas Christian Chust | 31 Jul 14:56

Re: Suggested patch for Open Dylan's C runtime

Bruce Hoult wrote:

> On 7/31/07, Thomas Christian Chust <chust <at> web.de> wrote:
>
>> Thomas Christian Chust wrote:
>>
>>> Bruce Hoult wrote:
>>>
>>>> You could allocate the list link for the object at the point that you
>>>> register the finalizer for it
>>
>> I guess I shouldn't hack such stuff together when in a hurry. A much
>> easier solution which also avoid allocation inside the finalization
>> procedure is possible by simply using the data argument to that
>> procedure to supply a preallocated cons cell.
> 
> Right. :-)  Looks good provided that link object won't get GC'd in the meantime.

But if that happened, I would consider it a bug in the garbage
collector. I my eyes, being the fixed parameter to a registered callback
fully qualifies an object as being alive.

cu,
Thomas

--

-- 
Gd-hackers mailing list
Gd-hackers <at> gwydiondylan.org
https://www.opendylan.org/mailman/listinfo/gd-hackers
(Continue reading)

Permalink | Reply |
headers
Bruce Hoult | 31 Jul 23:21

Re: Suggested patch for Open Dylan's C runtime

On 8/1/07, Thomas Christian Chust <chust <at> web.de> wrote:
> Bruce Hoult wrote:
> > Right. :-)  Looks good provided that link object won't get GC'd in the meantime.
>
> But if that happened, I would consider it a bug in the garbage
> collector. I my eyes, being the fixed parameter to a registered callback
> fully qualifies an object as being alive.

I agree.  My point is only that it's probably better to check that
this bug does not exist now, while you're thinking of it, than for
someone find it by accident later when it might take many hours to
figure out what is going on.
--

-- 
Gd-hackers mailing list
Gd-hackers <at> gwydiondylan.org
https://www.opendylan.org/mailman/listinfo/gd-hackers
Permalink | Reply |
headers
Thomas Christian Chust | 2 Aug 12:15

Re: Suggested patch for Open Dylan's C runtime

Bruce Hoult wrote:
> On 8/1/07, Thomas Christian Chust <chust <at> web.de> wrote:
>> Bruce Hoult wrote:
>>> Right. :-)  Looks good provided that link object won't get GC'd in the meantime.
>> But if that happened, I would consider it a bug in the garbage
>> collector. I my eyes, being the fixed parameter to a registered callback
>> fully qualifies an object as being alive.
> 
> I agree.  My point is only that it's probably better to check that
> this bug does not exist now, while you're thinking of it, than for
> someone find it by accident later when it might take many hours to
> figure out what is going on.

Well, all I can say is that the patch works fine on my Mac when I create
a large bunch of small transient objects registering for finalization in
their initialize method and draining the finalization queue either
afterwards or in the initialize method as well. I think that if there
were such a bug, it should show up in this situation -- if the test
objects are marked for collection, it is pretty likely that some of the
unused queue nodes should be marked too, if they were not considered
alive any longer.

Also the documentation of the Boehm garbage collector states in the
documentation for GC_register_finalizer_ignore_self and _no_order: "Note
that cd [the data argument for the registered finalizer] will still be
viewed as accessible, even if it refers to the object itself." I think
this confirms that the data argument is always stored somewhere
reachable from a GC root.

However I don't feel like digging into the garbage collector source and
(Continue reading)

Permalink | Reply |
headers
Bruce Hoult | 2 Aug 12:19

Re: Suggested patch for Open Dylan's C runtime

On 8/2/07, Thomas Christian Chust <chust <at> web.de> wrote:
> Also the documentation of the Boehm garbage collector states in the
> documentation for GC_register_finalizer_ignore_self and _no_order: "Note
> that cd [the data argument for the registered finalizer] will still be
> viewed as accessible, even if it refers to the object itself." I think
> this confirms that the data argument is always stored somewhere
> reachable from a GC root.

Cool :-)

I'm more than happy.
--

-- 
Gd-hackers mailing list
Gd-hackers <at> gwydiondylan.org
https://www.opendylan.org/mailman/listinfo/gd-hackers
Permalink | Reply |
headers
Hannes Mehnert | 2 Aug 16:45

Re: Suggested patch for Open Dylan's C runtime

Hi Bruce, Thomas,

Bruce Hoult wrote:
> On 8/2/07, Thomas Christian Chust <chust <at> web.de> wrote:
>> Also the documentation of the Boehm garbage collector states in the
>> documentation for GC_register_finalizer_ignore_self and _no_order: "Note
>> that cd [the data argument for the registered finalizer] will still be
>> viewed as accessible, even if it refers to the object itself." I think
>> this confirms that the data argument is always stored somewhere
>> reachable from a GC root.
> 
> Cool :-)
> 
> I'm more than happy.

great. I committed the updated patch yesterday. Thanks a lot for your
work, both of you.

Hannes
--

-- 
Gd-hackers mailing list
Gd-hackers <at> gwydiondylan.org
https://www.opendylan.org/mailman/listinfo/gd-hackers
Permalink | Reply |

Gmane