Hi Kay,
You are right for general usage of RefCounted... I should probably add
a comment to that class.
Solr as a whole doesn't have a problem though because other locking
ensures that once the refcount reaches 0, no one will increment it
again.
SolrCore holds a reference itself, and only calls decref in
SolrCore.registerSearcher() which is synchronized on searcherLock.
Once the refcount is 0, there will be another registered searcher that
will be handed out (incref()).
-Yonik
On 5/29/07, Kay Röpke <[EMAIL PROTECTED]> wrote:
Hi people!
As I was looking at the solr source code the other day, I noticed the
following:
public abstract class RefCounted<Type> {
[...]
public final RefCounted<Type> incref() { refcount.incrementAndGet
(); return this; }
public void decref() { if (refcount.decrementAndGet()==0) close(); }
protected abstract void close();
[...]
}
The implementation for close() supposedly invalidates the contents of
the field resource (in
this case a SolrIndexSearcher, but that's not the point).
I believe there is a race condition between incref() and decref().
Consider these events:
Thread A acquires an RefCounted object, refcount is 1
Thread A calls decref(), decrementAndGet() sets refcount to 0, "if"
condition is true
before Thread A enters close(), Thread B acquires the same instance
of the refcounted object
and calls incref().
Thread A increases refcount to 1 and returns the refcounted object
Thread B continues to close() and thus invalidates the resource which
Thread A now tries to use
things break.
Is my reasoning correct? My fear is that I slowly get paranoid ;)
If so (not the paranoid part), what would be your suggestion to fix
this? Adding locks
around incrementAndGet() and decrementAndGet(), close() somewhat
defeats the elegance
of RefCounted. One might play tricks with various checks of refcount,
but I haven't
completely thought that one through yet, so it might not work at all.
cheers,
-k
--
Kay Röpke
http://www.epublica.de
