RFR 9: 8138696 : java.lang.ref.Cleaner - an easy to use alternative to finalization
Peter Levart
peter.levart at gmail.com
Wed Dec 9 09:30:09 UTC 2015
On 12/09/2015 08:54 AM, David Holmes wrote:
> On 9/12/2015 5:05 PM, Peter Levart wrote:
>> Hi,
>>
>> I think the only way to try to prevent such things is with a good
>> example in javadoc that "screams" of possible miss-usages.
>
> Problem is that many people - myself included - would not have a clue
> when a lambda "captures this"! Is that part of lambda expressions or a
> consequence of the current implementation?
>
> David
I can't seem to find anything about capturing variables by lambda
expressions in JLS (except the talk about definitely assigned local
variables). I think lambdas ware meant to act like anonymous inner
classes. What they do is they capture a variable not the value variable
has at capture time. For locals this is the same, as lambdas and
anonymous inner classes only allow capturing "effectively final" and
"definitely assigned" local variables, which means only locals that
don't change the value after they are captured. With fields this is
different. Instance fields are never really final (even if declared
final). And as fields are implemented, they can not exist without the
object that contains them...
Regards, Peter
P.S. The difference between anonymous inner class creation expression
and lambda expression is that in instance scope, anonymous inner class
always "captures" the outer instance while lambda expression only if it
has to.
>
>>
>> public static class CleanerExample implements AutoCloseable {
>>
>> private static final Cleaner cleaner = ...; // preferably a
>> shared cleaner
>>
>> private final PrivateNativeResource pnr;
>>
>> private final Cleaner.Cleanable cleanable;
>>
>> public CleanerExample(args, ...) {
>>
>> // prepare captured state as local vars...
>> PrivateNativeResource _pnr = ...;
>>
>> this.cleanable = cleaner.register(this, () -> {
>> // DON'T capture any instance fields with lambda since
>> that would
>> // capture 'this' and prevent it from becoming
>> phantom-reachable!!!
>> _pnr.close();
>> });
>>
>> this.pnr = _pnr;
>> }
>>
>> public void close() {
>> cleanable.clean();
>> }
>>
>>
>> Regards, Peter
>>
>> On 12/09/2015 05:15 AM, Vitaly Davidovich wrote:
>>> This has the same problem, doesn't it? The bottom line is if the
>>> lambda is () -> <access a field in any manner> you're getting a
>>> capture of `this`.
>>>
>>> On Tue, Dec 8, 2015 at 5:08 PM, Roger Riggs <Roger.Riggs at oracle.com
>>> <mailto:Roger.Riggs at oracle.com>> wrote:
>>>
>>> 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
>>> <mailto: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/
>>>
>>> <http://cr.openjdk.java.net/%7Erriggs/webrev-cleaner-8138696/>
>>>
>>> [2]http://cr.openjdk.java.net/~rriggs/cleaner-doc/index.html
>>>
>>> <http://cr.openjdk.java.net/%7Erriggs/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