You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by Stefan Reich <st...@googlemail.com> on 2017/12/11 00:46:17 UTC
Possible problem with WeakIdentityHashMap
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
Re: Possible problem with WeakIdentityHashMap
Posted by Stefan Reich <st...@googlemail.com>.
No feedback? Anyway, NVM. Check out my page if you want to see the future
of programming (or lack thereof).
Cheers,
Stefan
On 11 December 2017 at 02:05, Stefan Reich <
stefan.reich.maker.of.eye@googlemail.com> wrote:
> 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.eye@
> 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/avr
>> o/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
>>
>
>
--
Stefan Reich
BotCompany.de
Re: Possible problem with WeakIdentityHashMap
Posted by Stefan Reich <st...@googlemail.com>.
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.eye@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
>