Re: Scarab commit: svn commit: r10567 -trunk/src/java/org/tigris/scarab: actions tools xmlrpc

>>Therefore the initial call to IssueSearchFactory must be within the try
>>block to ensure that, even if it crashes and throws an exception,
>>notifyDone() is called to finish with.
> 
> This may now reduce the counter too much, because when the
> IssueSearchFactory.getInstance fails, it is not clear, if the failure
> happened before decrementing the counter or after. So this solution
> isn't save either.


The code that increments the counter is inside register() which is the 
first thing called in IssueSearchFactory.getInstance so this is unlikely 
with, now i see, two large exceptions:
 when register() does fail to increment the counter because the resource 
limit as been reached and it throws a MaxConcurrentSearchException, or it 
were interrupted, and the caller still calls notifyDone. This problem was 
existing before my commit and i have fixed it in r10568 & r10569 (sorry 
about the two commits, that was clumsy).

>>Since the default maximum for numActive is 4 it will only take four such
>>occurrences before the application is /dead/ and can no longer execute
>>queries. 
> 
> Yes, that's a serious concern.
> IMO the solution is, to change IssueSearchFactory.getInstance, that it
> is sure that the counter was not incremented, when the method throws an
> exception and move the call of the method out of the catch-block again.

You can take this approach but it comes with an ugly burden in how you 
declare exceptions. 
That is, to implement "it is sure that the counter was not incremented" 
in IssueSearchFactory.getInstance means that the catch around the new 
IssueSearch(..) must catch *everything* decrement the count and then 
throw *everything* again. You either have to add "throws Throwable" to 
IssueSearchFactory.getInstance or rewrap all Errors into Exceptions.
Neither of these are acceptable imho.

It is far easier, at least AFAIK, to guarantee the counter was 
incremented.

However we decide the javadoc on IssueSearchFactory desperately needs to 
be improved to declare this contract.

~mck


-- 
"This is my simple religion. There is no need for temples; no need for
complicated philosophy. Our own brain, our own heart is our temple; the
philosophy is kindness." The Dalai Lama
| www.semb.wever.org | www.sesat.no | www.sesam.no |