On Wed, 10 Jul 2024 23:16:07 GMT, Joe Darcy <da...@openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/Object.java line 126:
>> 
>>> 124:      *
>>> 125:      * As as a quality of implementation concern, a particular
>>> 126:      * implementation of this method may or may not support generating
>> 
>> "may not support generating hash codes" sounds weird; maybe like "may or may 
>> not guard against cyclic data structures in recursive hash code generation."
>> 
>> I think the key is "recursive" here, as only recursive implementations are 
>> at risk of these faults.
>
> Well, recursion is the easiest way to accidentally write an infinite loop, 
> but I think a few other ones would be possible too ;-) I'll considering 
> updating the wording to highlight the most likely hazards; thanks.

Unfortunately the typical implementation pattern is to recur through the 
component fields of the object. Not recursion through `this` but rather the 
hashCode implementation of some object will typically invoke hashCode of its 
component fields. Of course this can result in infinite recursion (really, 
StackOverflowError) if the object graph contains cycles, but in general, 
implementations of these methods don't guard against this possibility. (Nor 
should they. Adding such guard code seems impractical.)

Note that the default implementations of these methods for records and for 
Collection classes doesn't guard against cycles. (The collections contain some 
special cases for collection that contains itself, but not cycles in general.)

I have a general concern about advice that isn't actionable, such as "may not 
support" or "care should be taken" etc. If we really want to say something 
about cyclic data structures, maybe we could say something like this. For data 
structures that may contain cycles, these implementations should avoid 
traversing component fields that may lead to cycles. A class could simply 
inherit the method implementations from Object. Alternatively, the class could 
contain a unique ID that's used for determining the result of 
equals/hashCode/toString. (Of course, determining uniqueness is up to the 
application.)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20128#discussion_r1673238207

Reply via email to