RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization
Vitaly Davidovich
vitalyd at gmail.com
Tue Dec 8 23:09:44 UTC 2015
Can we enhance java to allow specifying lambda capture and how the value is
captured? :)
This is very subtle to the point where I maybe wouldn't even encourage use
of lambda in this context.
sent from my phone
On Dec 8, 2015 4:25 PM, "Peter Levart" <peter.levart at gmail.com> 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