Explainer: Clarify when the finalization callback is called #148
Conversation
| @@ -126,7 +128,8 @@ This example shows usage of the whole `FinalizationGroup` API: | |||
| - The object whose lifetime we're concerned with. Here, that's `this`, the `FileStream` object. | |||
| - A “holdings” value, which is used to represent that object when cleaning it up in the finalizer. Here, the holdings are the underlying `File` object. | |||
| - An unregistration token, which is passed to the `unregister` method when the finalizer is no longer needed. Here we use `this`, the `FileStream` object itself, since `FinalizationGroup` doesn't hold a strong reference to the unregister token. | |||
| - The `FinalizationGroup` constructor is called with a callback as an argument. This callback is called with an iterator of the holdings values. | |||
| - The `FinalizationGroup` constructor is called with a callback as an argument. This callback is called after a garbage collection occurred and at least one registered object was collected. The finalizer callback is called with an iterator of the holdings values for the registered objects that were collected.\ | |||
There was a problem hiding this comment.
- The `FinalizationGroup` constructor is called with a callback as an argument.
+ The `FinalizationGroup` constructor must be called with a finalizer callback as an argument.- This callback is called after a garbage collection occurred and at least one registered object was collected.
+ This callback may be called after a garbage collection occurred and at least one registered object was collected.- The finalizer callback is called with an iterator of the holdings values for the registered objects that were collected.
+ The finalizer callback is called with a dynamic iterator of the holdings values for each of the collected objects registered in the corresponding instance.Is the \ at the end intentional?
There was a problem hiding this comment.
the part were it says the callback may be called after a gc reflects the current normative text as we can't assert if the callback will be called. This reflects an important aspect of the feature as a consecutive GC can't trigger the callback anymore if the object was collected before, in any previous GC.
There was a problem hiding this comment.
For a collecting engine, if an object registered with the FinalizationGroup is collected, the callback will be called, except if cleanupSome is called between collection and scheduled callback invocation, and the iterator is consumed (or if process is terminated, of course).
I don't think consecutive GC changes this. The callback will be invoked once.
Now we can probably clarify that the callback is invoked only if an object is newly collected, not for previously collected objects which holdings were not consumed. I don't think "may be called" clarifies that enough. Maybe replacing was collected by has just been collected would help?
We could probably clarify the interaction of iterators and collection executions, adding something like this to the following paragraph:
An object stays registered with the FinalizationGroup until it's either explicitly unregistered, or when its holdings value is yielded by the iterator after being collected. If the finalizer callback doesn't consume the iterator, the collected object stays registered, and the holdings value will be available in a future iterator. However, the callback is only called when registered objects are newly collected, it won't be triggered for previously collected objects alone.
The application can perform cleanup for collected objects whose holdings values weren't processed yet by callingcleanupSome().
There was a problem hiding this comment.
if an object registered with the FinalizationGroup is collected, the callback will be called,
for what the spec says, callback may be called or not.
I don't think consecutive GC changes this.
consecutive GC can't make the callback be called for the previously collected references. So it's the 1 chance opportunity.
The callback will be invoked once.
once or zero times. Per spec text.
Now we can probably clarify that the callback is invoked only if an object is newly collected, not for previously collected objects which holdings were not consumed.
Yes, with the nit of "callback may be invoked".
Maybe replacing was collected by has just been collected would help?
sure.
The example looks good, the only nit picking part is:
the callback is only called when...
that is actually:
the callback might only be called when ...
The only goal I have is to reflect the spec text, even if a specific implementation will secure it being called. It remains optional.
| @@ -126,7 +128,8 @@ This example shows usage of the whole `FinalizationGroup` API: | |||
| - The object whose lifetime we're concerned with. Here, that's `this`, the `FileStream` object. | |||
| - A “holdings” value, which is used to represent that object when cleaning it up in the finalizer. Here, the holdings are the underlying `File` object. | |||
| - An unregistration token, which is passed to the `unregister` method when the finalizer is no longer needed. Here we use `this`, the `FileStream` object itself, since `FinalizationGroup` doesn't hold a strong reference to the unregister token. | |||
| - The `FinalizationGroup` constructor is called with a callback as an argument. This callback is called with an iterator of the holdings values. | |||
| - The `FinalizationGroup` constructor is called with a callback as an argument. This callback is called after a garbage collection occurred and at least one registered object was collected. The finalizer callback is called with an iterator of the holdings values for the registered objects that were collected.\ | |||
| In this case a single FinalizationGroup is shared by all `File` objects as a static field of the `File` class. The class' `#cleanUp` callback will be invoked when any `File` instance was garbage collected. | |||
There was a problem hiding this comment.
- In this case a single FinalizationGroup is shared by all `File` objects as a static field of the `File` class. The class' `#cleanUp` callback will be invoked when any `File` instance was garbage collected.
+ In this case a single FinalizationGroup is shared by all `File` objects as a static field of the `File` class. The class' `#cleanUp` callback may be invoked when any `File` instance was garbage collected.
|
@leobalter, I'm just now looking at your comments. I'm wondering if in the explainer we have to be walking on egg shells everytime about the fact the finalizer callback may not be invoked. Realistically, if the engine collects garbage, the callback will usually be invoked when an object is collected. |
|
I don't think it needs to walk on egg shells, but for me I'd prefer if the readme reflects the spec text, not any specific implementation. This helps highlighting the optional parts of the specs, which is important. I honestly believe that GC should remains optional, but I honestly don't see a blocker for this optional point where the callback may be called. My suggestion:
It fits existing implementation. I could make a PR for this proposed change if you're interested in following up with this suggestion. Otherwise, I don't want to block this PR, it's the README and it does not affect normative text anyway. I appreciate the convo, thou. |
Yeah I agree it's seems weird an implementation would be able to clear a WeakRef which is observable by the program, yet choose to not invoke the callback to notify the program. #147 already clarifies an implementation must invoke the callback for FinalizationGroups regardless of their liveness. Maybe we need a dedicated section in the spec that puts conditions on the optionality of the callback invocation.
I'll adjust this PR to capture that the callback may not always be invoked. We can works on the spec improvements in another. |
|
@mhofman Are you planning on addressing @leobalter 's comments? |
|
Sorry it totally skipped my mind. Hoping to have some time later this week. |
|
@mhofman Ping, are these comments being addressed? |
The FinalizationGroup naming can be understood as a the single finalization of a group of objects.
It wasn't clear in the explainer that the finalizer is simply shared by a group of objects and that the callback is called after any object registered with the group is collected.
See #66 (comment)