Ah, I think I found out why it's not a problem. The spurious IdentityWeakReference instances are indeed added to the queue. But once they are ready for reaping, they are set to null already. So no live objects can ever get lost.
Still, it would be a tiny efficiency win not to add references to the queue unnecessarily. For example, the IdentityWeakReference made in the remove() method can always be queue-less. Cheers, Stefan BotCompany.de On 11 December 2017 at 01:46, Stefan Reich < stefan.reich.maker.of....@googlemail.com> wrote: > Hi, I'm from the "Fix Java" department. > > This is an advanced problem, please bear with me. > > The issue concerns WeakIdentityHashMap (https://github.com/apache/ > avro/blob/master/lang/java/avro/src/main/java/org/apache/ > avro/util/WeakIdentityHashMap.java). I believe there may be a bug. > > Suppose the remove() function is called: > > public V remove(Object key) { > reap(); > return backingStore.remove(new IdentityWeakReference(key)); > } > > As you can see, an instance of IdentityWeakReference is made in the > process, and it registers itself against the ReferenceQueue. Herein lies > the problem. > > The IdentityWeakReference is added to the queue, but not to the map, so it > shouldn't be in the queue either. I think this creates a problem in the > reap() method later. > > Let's suppose the object was not in the map at the time of calling > remove(). Later, though, a matching key is inserted. THEN the reap() method > gets called, finds the spuriously added IdentityWeakReference and deletes > an object that shouldn't be deleted. > > Haha. Does anyone get any of this? As I said, it's advanced - but then > again, you guys write these classes. > > I think It is possible to fix this issue by creating IdentityWeakReference > objects without a queue in carefully selected places. > > First we might need to produce a failing test case. Anyone interested? > > Many greetings, > > Stefan Reich > BotCompany.de >
