RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization
Roger Riggs
Roger.Riggs at Oracle.com
Tue Dec 8 22:08:04 UTC 2015
Hi,
Another option that should always capture is to define a specific static
method with the needed values as arguments:
public static class CleanerExample implements AutoCloseable {
FileDescriptor fd = ...;
private static final Cleaner cleaner = Cleaner.create();
private final Cleaner.Cleanable cleanable =
cleaner.register(this,*() -> cleanup(fd)*);
@Override
public void close() {
cleanable.clean();
}
*static void cleanup(FileDescriptor fd ...) {**
** fd.close();**
** }*
}
On 12/8/2015 4:24 PM, Peter Levart wrote:
>
>
> On 12/08/2015 08:08 PM, Steven Schlansker wrote:
>> On Dec 8, 2015, at 10:51 AM, Peter Levart<peter.levart at gmail.com> wrote:
>>> On 12/08/2015 04:34 PM, Roger Riggs wrote:
>>>> private final Cleaner.Cleanable cleanable = cleaner.register(this, () -> fd.close());
>>> Sorry Roger, but this example is flawed. This is tricky! The lambda "() -> fd.close()" captures 'this', not only 'fd' as can be seen by running the following example:
>>> To correct that, but still use lambda, you would have to capture a local variable
>> It looks like using "fd::close" might also work, and is more concise:
>>
>> IntSupplier x = () -> 10;
>> IntSupplier xS = x::getAsInt;
>>
>> @Test
>> public void test() {
>> System.out.println(xS.getAsInt());
>> x = () -> 15;
>> System.out.println(xS.getAsInt());
>> }
>>
>> 10
>> 10
>
> Yes, good idea. This is a pre-bound method reference (the part on the
> left of '::' is evaluated immediately). Contrast to lambda, where
> "fd.close()" is an expression in the lambda body which is evaluated
> when lambda is invoked and that expression is composed of a field get
> + method invocation. In order to get an instance field, the object
> containing it must be captured.
>
> So for Roger's example, this will work:
>
>
> public static class CleanerExample implements AutoCloseable {
>
> FileDescriptor fd = ...;
>
> private static final Cleaner cleaner = Cleaner.create();
>
> private final Cleaner.Cleanable cleanable =
> cleaner.register(this, fd::close);
>
> @Override
> public void close() {
> cleanable.clean();
> }
> }
>
>
> ...if FileDescriptor.close() is an instance method with no arguments
> and doesn't throw any checked exceptions.
>
>
> Regards, Peter
>
>>>> I'll work the example into the javadoc.
>>>>
>>>> Roger
>>>>
>>>>
>>>> On 12/08/2015 07:25 AM, Peter Levart wrote:
>>>>> On 12/08/2015 09:22 AM, David Holmes wrote:
>>>>>> Actually I'm having more doubts about this API.
>>>>>>
>>>>>> Library writers use finalize() as a last ditch cleanup mechanism in
>>>>>> case the user doesn't explicitly call any "cleanup" method. So as a
>>>>>> library writer I would think I am now expected to register my
>>>>>> instances with a Cleaner and provide a Runnable that does what
>>>>>> finalize() would have done. But in that usage pattern the end user of
>>>>>> my objects never has any access to my Cleanables so can never call
>>>>>> clean() themselves - instead they should be calling the cleanup
>>>>>> function directly, just as they would previously. So the whole "invoke
>>>>>> at most once" for the clean() method seems somewhat unnecessary; and
>>>>>> the way we should write the cleanup method and the Runnable need to be
>>>>>> more cleary explained as the idempotentcy of the cleanup needs to be
>>>>>> handled in the library writers code not the Cleaner/Clenable
>>>>>> implementation.
>>>>>>
>>>>>> David
>>>>> Hi David, (once again for the list)
>>>>>
>>>>> I agree that an example would be most helpful. Here's how a normal finalizable object is typically coded:
>>>>>
>>>>> public class FinalizeExample implements AutoCloseable {
>>>>>
>>>>> private boolean closed;
>>>>>
>>>>> @Override
>>>>> public synchronized void close() {
>>>>> if (!closed) {
>>>>> closed = true;
>>>>> // cleanup actions accessing state of FinalizeExample, executed at most once
>>>>> }
>>>>> }
>>>>>
>>>>> @Override
>>>>> protected void finalize() throws Throwable {
>>>>> close();
>>>>> }
>>>>> }
>>>>>
>>>>>
>>>>> Re-factoring to use Cleaner is a process that extracts the state representing native resource from the user-facing class into a private nested static class and makes the user-facing object just a facade that has access to the state object and is registered with a Cleaner. The Cleaner.Cleanable instance is also made accessible from the user-facing object, so it can provide the on-demand cleaning:
>>>>>
>>>>> public static class CleanerExample implements AutoCloseable {
>>>>>
>>>>> private static class State implements Runnable {
>>>>> @Override
>>>>> public void run() {
>>>>> // cleanup actions accessing State, executed at most once
>>>>> }
>>>>> }
>>>>>
>>>>> private static final Cleaner cleaner = Cleaner.create();
>>>>>
>>>>> private final State state = new State();
>>>>> private final Cleaner.Cleanable cleanable = cleaner.register(this, state);
>>>>>
>>>>> @Override
>>>>> public void close() {
>>>>> cleanable.clean();
>>>>> }
>>>>> }
>>>>>
>>>>>
>>>>> Regards, Peter
>>>>>
>>>>>> On 8/12/2015 6:09 PM, David Holmes wrote:
>>>>>>> Hi Roger,
>>>>>>>
>>>>>>> Sorry I had no choice but to look at this more closely ... and apologies
>>>>>>> as this is very late feedback ... I only looked at the API not the
>>>>>>> details of the implementation.
>>>>>>>
>>>>>>> On 8/12/2015 4:50 AM, Roger Riggs wrote:
>>>>>>>> Hi David,
>>>>>>>>
>>>>>>>> Thanks for the comments,
>>>>>>>>
>>>>>>>> Updated the javadoc and webrev with editorial changes.
>>>>>>>>
>>>>>>>> [1]http://cr.openjdk.java.net/~rriggs/webrev-cleaner-8138696/
>>>>>>>> [2]http://cr.openjdk.java.net/~rriggs/cleaner-doc/index.html
>>>>>>> Should cleaning and cleanables be mentioned as part of the package-doc
>>>>>>> for java.lang.ref? Else they seem to be an overlooked add-on not part of
>>>>>>> the core reference related functionality. Perhaps even state how they
>>>>>>> are preferred to use of finalization?
>>>>>>>
>>>>>>> Cleaner.Cleanable:
>>>>>>>
>>>>>>> It was unclear to me what the usage model was for this. I'm assuming
>>>>>>> that the intent is that rather than register a "thunk" (lets call it an
>>>>>>> "action") that can be invoked directly by user-code, the user should
>>>>>>> invoke the action via the call to clean(). In which case I think it
>>>>>>> should be explained somewhat more clearly - see below.
>>>>>>>
>>>>>>> I would describe the Cleanable class as:
>>>>>>>
>>>>>>> Cleanable: Represents an object that has been registered for cleanup by
>>>>>>> a Cleaner. The object can be cleaned directly, by a call to clean(), if
>>>>>>> it is no longer to be used, else it will be cleaned automatically when
>>>>>>> the object becomes phantom-reachable.
>>>>>>>
>>>>>>> Cleanable.clean: Unregisters this Cleanable and performs the cleanup
>>>>>>> action that was associated with it. If this Cleanable has already been
>>>>>>> unregistered nothing happens. The cleanup action is invoked at most once
>>>>>>> per registered Cleanable, regardless of the number of calls to clean().
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> Looking at Cleaner ....
>>>>>>>
>>>>>>>
>>>>>>> "Cleaner manages a set of object references and corresponding cleaning
>>>>>>> functions"
>>>>>>>
>>>>>>> I would say "cleaning actions" rather than functions as they yield no
>>>>>>> value. This change needs to be made throughout.
>>>>>>>
>>>>>>> "The most efficient use is to explicitly invoke the clean method when
>>>>>>> the object is closed or no longer needed. The cleaning function is a
>>>>>>> Runnable to be invoked at most once when the object is no longer
>>>>>>> reachable unless it has already been explicitly cleaned."
>>>>>>>
>>>>>>> To me this doesn't quite capture the need to not use the Runnable
>>>>>>> directly. I would rephrase:
>>>>>>>
>>>>>>> "In normal use a object should be cleaned up when no longer required, by
>>>>>>> invoking the clean() method of the associated Cleanable. This guarantees
>>>>>>> that the cleaning action will be performed at most once per object:
>>>>>>> either explicitly, or automatically if it becomes phantom-reachable. If
>>>>>>> cleaned explicitly the object should not be used again. Note that the
>>>>>>> cleaning action must not refer to the object ..."
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> Question: what happens if an object is registered simultaneously with
>>>>>>> multiple Cleaners? Do we need to warn the user against that?
>>>>>>>
>>>>>>> ---
>>>>>>>
>>>>>>> The phrase "process the unreachable objects and to invoke cleaning
>>>>>>> functions" doesn't quite seem right to me. The objects themselves are
>>>>>>> never processed, or even touched - right? So really the thread is
>>>>>>> started to "invoke the cleanup actions for unreachable objects".
>>>>>>>
>>>>>>> create(): can also throw SecurityException if not allowed to
>>>>>>> create/start threads.
>>>>>>>
>>>>>>> register(Object obj, Runnable thunk): thunk -> action
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>>
>>>>>>>
>>>>>>>> On 12/6/15 7:46 PM, David Holmes wrote:
>>>>>>>>> Hi Roger,
>>>>>>>>>
>>>>>>>>> Sorry to be late here but was trying not to get involved :)
>>>>>>>>>
>>>>>>>>> It is already implicit that ThreadFactory.newThread should return
>>>>>>>>> unstarted threads - that is what a new Thread is - so I don't think
>>>>>>>>> IllegalThreadStateException needs to be documented here as it is
>>>>>>>>> documenting behaviour of a broken ThreadFactory (and a broken
>>>>>>>>> ThreadFactory could throw anything) .
>>>>>>>> It does seem that new is fairly well understood but one can read of
>>>>>>>> ThreadFactory is as a bit ambiguous, lacking a direct reference to the
>>>>>>>> Thread.State of the new thread
>>>>>>>> and since it allows various attributes of the thread to be modified
>>>>>>>> after the constructor.
>>>>>>>> Since the runnable is supplied as an argument it is ready to be
>>>>>>>> started,
>>>>>>>> why not.
>>>>>>>> It seemed useful to reinforce the salient points.
>>>>>>>>> Also the no-arg cleaner() can also throw SecurityException.
>>>>>>>> The thread construction is done in doPriv so it should not throw.
>>>>>>>> Did I miss some edge case?
>>>>>>>>> Also this:
>>>>>>>>>
>>>>>>>>> 127 * On each call the {@link ThreadFactory#newThread(Runnable)
>>>>>>>>> thread factory}
>>>>>>>>> 128 * should return a {@link Thread.State#NEW new thread} with
>>>>>>>>> an appropriate
>>>>>>>>> 129 * {@linkplain Thread#getContextClassLoader context class
>>>>>>>>> loader},
>>>>>>>>> 130 * {@linkplain Thread#getName() name},
>>>>>>>>> 131 * {@linkplain Thread#getPriority() priority},
>>>>>>>>> 132 * permissions, etc.
>>>>>>>>>
>>>>>>>>> then begs the questions as to what is "appropriate". I think this can
>>>>>>>>> be said much more simply as: "The ThreadFactory must provide a Thread
>>>>>>>>> that is suitable for performing the cleaning work". Though even that
>>>>>>>>> raises questions. I'm not sure why a ThreadFactory is actually needed
>>>>>>>>> here ?? Special security context? If so that could be mentioned, but I
>>>>>>>>> don't think name or priority need to be discussed.
>>>>>>>> It was intended to prod the client to be deliberate about the
>>>>>>>> threadFactory.
>>>>>>>> Since the client is integrating the Cleaner and respective cleaning
>>>>>>>> functions
>>>>>>>> with the client code, the ThreadFactory makes it possible for the
>>>>>>>> client to
>>>>>>>> initialize a suitable thread and the comments serve as a reminder.
>>>>>>>> I agree that the phrase 'suitable for performing the cleaning work' is
>>>>>>>> the operative one.
>>>>>>>>
>>>>>>>> Thanks, Roger
>>>>>>>>> Thanks,
>>>>>>>>> David
>>>>>>>>>
>
More information about the core-libs-dev
mailing list