Agree entirely with Andrew's concern. I would mostly point to the java.lang.Reference documentation. One possible approach follows.
Add the following to the apiNote. I would add it at the beginning, since I've rarely encountered the inheritance issues, and they're probably easier to debug: As with all reachability-based cleanup mechanisms, it is important to use java.lang.reReference.reachabilityFence to ensure that objects remain reachable while resources embedded in the object are in use. However, looking at the reachabilityFence documentation, I think it's quite misleading, and greatly understates the need for using it. In particular the following paragraph seems most misleading: "This method is designed for use in uncommon situations of premature finalization where using synchronized blocks or methods, or using other synchronization facilities are not possible or do not provide the desired control. This method is applicable only when reclamation may have visible effects, which is possible for objects with finalizers (See Section 12.6 17 of The Java™ Language Specification) that are implemented in ways that rely on ordering control for correctness." These seem completely misleading to me. I would replace this with This method should be used when cleanup actions (finalize() calls, or Reference enqueuing, Cleaner invocation) could otherwise be triggered while a resource under control of the object is still in use. This method should normally be called when these cleanup facilities are used to perform actions other than simply issuing a warning. (Specific complaints about the original: In my experience, this is needed for the large majority of finalization/ReferenceQueue uses. What's the point of a finalizer without visible effects? The object is otherwise dead: Why bother? In practice, synchronization almost never provides the correct guarantees. Finalizers should indeed usually synchronize, but NOT on the object itself. It's not clear what "ordering control" here means. The order in which program actions are performed generally does matter.) The section starting with "It is sometimes possible to better encapsulate use of reachabilityFence..." just seems misleading. Delete it. I can't imagine a scenario in which that could work. Using files as a (bad for other reasons) example, assume the external resource is a file descriptor, and I'm about to read it. Keeping the file open until I retrieve the descriptor isn't good enough. It needs to be kept open for the duration of the read. I would replace the last paragraph with the following, or delete it entirely. I don't believe I've ever seen code to which it applies: ReachabilityFence is not needed when all uses of the embedded resource (including in the constructors or the finalize() method) are entirely enclosed in synchronized(this) blocks. (Note again that surrounding all accesses to the object itself in synchronized blocks does not suffice.) On Fri, Mar 31, 2017 at 10:29 AM, Stuart Marks <stuart.ma...@oracle.com> wrote: > > > On 3/31/17 6:55 AM, Roger Riggs wrote: > >> The webrev for deprecating finalize has been updated: >> - to improve the advice existing JDK subclasses overriding finalize >> provides in >> @deprecated javadoc, >> - to expand Object.finalize() javadoc to reinforce the correct use of >> super.finalize() by subclasses, >> - and to remove redundant @SuppressWarnings annotations. >> >> Please review and comment. >> >> Webrev: >> http://cr.openjdk.java.net/~rriggs/webrev-finalize-deprecate-8165641/ >> >> Issue: >> https://bugs.openjdk.java.net/browse/JDK-8165641 >> > > Hi Roger, > > The main text in Object.finalize(), and the text for each of the > occurrences of an overriding finalize() method, look good. > > I don't think j.l.Enum.finalize() needs to be deprecated. The main point > of deprecating finalize() is to inform subclassers. But Enum.finalize() > can't be overriden. I suppose somebody could *call* it from an enum, and > deprecation would serve to generate a warning, but this is pretty obscure. > > I'd suggest replacing the deprecation of Enum.finalize() with > @SuppressWarnings. > > s'marks >
