{"action":"opened","issue":{"url":"https://api.github.com/repos/immutables/immutables/issues/867","repository_url":"https://api.github.com/repos/immutables/immutables","labels_url":"https://api.github.com/repos/immutables/immutables/issues/867/labels{/name}","comments_url":"https://api.github.com/repos/immutables/immutables/issues/867/comments","events_url":"https://api.github.com/repos/immutables/immutables/issues/867/events","html_url":"https://github.com/immutables/immutables/issues/867","id":385766924,"node_id":"MDU6SXNzdWUzODU3NjY5MjQ=","number":867,"title":"Weak Interning should not use identity comparison in equals()","user":{"login":"argibbs","id":4406514,"node_id":"MDQ6VXNlcjQ0MDY1MTQ=","avatar_url":"https://avatars2.githubusercontent.com/u/4406514?v=4","gravatar_id":"","url":"https://api.github.com/users/argibbs","html_url":"https://github.com/argibbs","followers_url":"https://api.github.com/users/argibbs/followers","following_url":"https://api.github.com/users/argibbs/following{/other_user}","gists_url":"https://api.github.com/users/argibbs/gists{/gist_id}","starred_url":"https://api.github.com/users/argibbs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/argibbs/subscriptions","organizations_url":"https://api.github.com/users/argibbs/orgs","repos_url":"https://api.github.com/users/argibbs/repos","events_url":"https://api.github.com/users/argibbs/events{/privacy}","received_events_url":"https://api.github.com/users/argibbs/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":0,"created_at":"2018-11-29T14:39:17Z","updated_at":"2018-11-29T14:39:17Z","closed_at":null,"author_association":"NONE","body":"# Problem Description\r\n\r\nIf interning is turned on, then the code for equals() looks like\r\n\r\n```java\r\n  public boolean equals(@Nullable Object another) {\r\n    return this == another;\r\n  }\r\n```\r\n\r\nThis is fine for strong interning, but for weak interning it's incorrect. Assume you have an object like:\r\n\r\n```java\r\nValue.Immutable(intern = true)\r\n@Value.Style(weakInterning = true)\r\npublic abstract class Foo {\r\n    public abstract String id();\r\n}\r\n```\r\n\r\nIf I create a value using id `\"some-string\"` then this is weakly interned. Subsequent calls will return the same instance, *unless the garbage collector intervenes*. At some point the garbage collector may decide to collect the entry from the weakly-keyed interner. After the GC runs, calls to create an `ImmutableFoo` using `\"some-string\"` will now return a different instance to before.\r\n\r\nHowever, even though the GC has cleaned up the interner, it's still possible that some other part of the code is hanging on to the original instance of the ImmutableFoo. Thus it's possible to get two different instances of the ImmutableFoo class. They will have the same id, but `equals` will fail, because it's comparing by instance.\r\n\r\n# Proposed Solution\r\nChange the `equals` implementations to use the \"non-interned\" equality check, i.e. comparing all non-auxiliary members. This is only necessary for weak interning. Strong interning can continue to use identity checks.\r\n\r\nFor the example above, the equals method should instead look something like:\r\n```java\r\n  public boolean equals(@Nullable Object another) {\r\n    if (this == another) return true;\r\n    return another instanceof ImmutableBarrid\r\n        && equalTo((ImmutableBarrid) another);\r\n  }\r\n```\r\n"}}
{"actor":{"display_login":"argibbs"}}