Sorry for the delay about this. I had a look at the patch some time ago
but didn’t have enough time to apply it.
I noticed that a NPE occurs when adding the following piece of code to
PropertyCacheTestCase and running it:
private final PropertyCache<Integer> cache = new PropertyCache<Integer>();
private class CacheFiller implements Runnable {
private final int start;
CacheFiller(int start) {
this.start = start;
}
public void run() {
for (int i = 0; i < 1000000; i++) {
cache.fetch(new Integer(start + i));
}
}
}
public void testCleanUp() throws InterruptedException {
Thread t1 = new Thread(new CacheFiller(0));
Thread t2 = new Thread(new CacheFiller(10000));
t1.start();
t2.start();
t1.join();
t2.join();
}
Here’s the stacktrace:
Exception in thread "Thread-1" java.lang.NullPointerException
at
org.apache.fop.fo.properties.PropertyCache.cleanReclaimedMapEntries(PropertyCache.java:153)
at
org.apache.fop.fo.properties.PropertyCache.fetch(PropertyCache.java:105)
at
org.apache.fop.fo.properties.PropertyCacheTestCase$CacheFiller.run(PropertyCacheTestCase.java:68)
at java.lang.Thread.run(Thread.java:595)
It happens when running the test with a Sun 1.5 JRE but apparently not
with OpenJDK 1.6. I’m still unsure about what’s happening.
It would be good to update the test cases to JUnit 4. Also, there are
some clean-ups and improvements to bring. Among other things, symmetry
is not properly tested in FObjectTest (symmetric.equals(getSut()) should
also be run); the tests for reflexivity, symmetry and transitivity
should be put in separate methods; testFailureCases is misnamed; etc.
Little things but that take some time to fix.
Vincent
On 28/02/12 17:09, mehdi houshmand wrote:
> Hi Guys,
>
> My apologies for the lack of transparency on this issue, but I didn't
> actually review the changes you made here, in fact, I barely looked at
> what PropertyCache actually does. I had some free time, and added a
> bunch of unit tests.
>
> The reason this hasn't been committed yet was because Vincent said he
> had some questions about the patch. That's as far as I know, maybe he
> could give some feedback on the issue.
>
> Let me reiterate my apologies again on this, it's not fair that this
> has been ignored. I'll endeavour to make the process more transparent
> in future, I hope this doesn't prevent you or any other contributors
> from submitting patches.
>
> Mehdi
>
>
> On 28 February 2012 16:52, Glenn Adams <gl...@skynav.com> wrote:
>> I support committing this patch, however I don't see an ICLA listed at [1]
>> for Alexios. Alexios, if you have not submitted an ICLA [2], please do so.
>>
>> I would be happy to apply the patch (if Mehdi doesn't have the time).
>>
>> [1] http://people.apache.org/committer-index.html#unlistedclas
>> [2] http://www.apache.org/licenses/icla.txt
>>
>>
>> On Tue, Feb 28, 2012 at 6:27 AM, Alexios Giotis <alex.gio...@gmail.com>
>> wrote:
>>>
>>> Hi,
>>>
>>> About 6 months ago, I had a deadlock issue that regularly stopped
>>> production servers. While I was opening a bugzilla ticket, I found that this
>>> was already reported back in 2009. This issue is still opened as it was
>>> difficult to reproduce. On that issue, I added:
>>>
>>> [1] An explanation of why a deadlock is possible.
>>> [1] Stacktraces of deadlocked threads from a production server.
>>> [2] A small unit test that adds a Thread.sleep() to the PropertyCache to
>>> make it always reproducable.
>>> [3] A patch solving this issue.
>>> [4] Explanations of why the patch rewrites the existing PropertyCache
>>> class.
>>>
>>> This was then reviewed and unit tests were added [5]. On top of this, I
>>> have committed the fix in my private branch and it works well on several big
>>> production systems. This is as far as I can go before a FOP committer takes
>>> it further. I am writing this because:
>>>
>>> - Deadlocks should be fixed. When they occur, there is no way around them.
>>> - The trunk is moving, the patch is aging and it will be more difficult to
>>> apply it over time.
>>> - It is discouraging for submitting more patches.
>>>
>>>
>>> Alexios Giotis
>>>
>>>
>>>
>>>
>>> [1] https://issues.apache.org/bugzilla/show_bug.cgi?id=46962#c3
>>> [2] https://issues.apache.org/bugzilla/attachment.cgi?id=27342
>>> [3] https://issues.apache.org/bugzilla/attachment.cgi?id=27477&action=diff
>>> [4] https://issues.apache.org/bugzilla/show_bug.cgi?id=46962#c7
>>> [5] https://issues.apache.org/bugzilla/show_bug.cgi?id=46962#c9
>>>
>>
