Skip to main content

Please review fix for 6794986 (BinaryTestFinder locks index file)

11 replies [Last post]
kromanovs
Offline
Joined: 2007-11-14

Fix for 6794986 "BinaryTestFinder locks binary data file in some cases".

This modification fixes occurrences, when BinaryTestFinder locked index file without starting to read from it. Previously, ZipFile was opened in various .init() methods, but closed only once in .readBinaryFile(). This means that if there was an .init() call with no consequent .read() - file remains locked.

Main change is in the openBinaryFile() method that now has parameter: 'boolean closeIfSuccess'. If it's set to true, implementation will close ZipFile once its integrity is verified.

Change: https://jtharness.dev.java.net/source/browse/jtharness?rev=1272&view=rev

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
kromanovs
Offline
Joined: 2007-11-14

Accepting the failure of the fix due to NPE and performance impact, I'd like to highlight that there were no changes in threading code in scope of this fix.

So, complaint about reading binary tree index in AWT-EventQueue, should be treated as a separate issue against the Harness.

kromanovs
Offline
Joined: 2007-11-14

I reproduced issues on noted TestSuite - both NPE and performance impact. it appeared that one boolean flag hasn't been cleared properly that caused excessive calls to file opening procedures on each .scan() invocation.

Please review updated fix at: https://jtharness.dev.java.net/source/browse/jtharness?rev=1289&view=rev

Fix has been tested on noted TestSuite and it worked well.

sergey_borodin
Offline
Joined: 2006-10-20

I've tested proposed fix on testsuite with big number of tests and binary cache. No NPE occured. So, I'm agree with fix.

Sergey

kromanovs
Offline
Joined: 2007-11-14

2nd version of the fix is committed to jt4.2_b12

kromanovs
Offline
Joined: 2007-11-14

Re-opening a thread due to failure of previous fix

kromanovs
Offline
Joined: 2007-11-14

Fix is integrated into trunk/ and branches/code/jth42-treeint

bkurotsu
Offline
Joined: 2004-12-13

I didn't test it, but it looked ok. I did see that it had a wildcard import at the top, which we dont allow. If you didn't change it, can you please change the import java.io.*; to identify exactly which classes are needed?

Thanks!

bkurotsu
Offline
Joined: 2004-12-13

There seems to be a defect. I opened a test suite with a single large JTD file. It produced this exception before it finished loading tests:
Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException
at com.sun.javatest.finder.BinaryTestFinder.readBinaryFile(BinaryTestFinder.java:332)
at com.sun.javatest.finder.BinaryTestFinder.scan(BinaryTestFinder.java:192)
at com.sun.javatest.TestFinder.read(TestFinder.java:439)
at com.sun.javatest.TRT_TreeNode.processFile(TRT_TreeNode.java:1076)
at com.sun.javatest.TRT_TreeNode.scanIfNeeded(TRT_TreeNode.java:639)
at com.sun.javatest.TRT_TreeNode.getTreeNodes(TRT_TreeNode.java:169)
at com.sun.javatest.exec.TT_BasicNode.updateNode0(TT_BasicNode.java:319)
at com.sun.javatest.exec.TT_BasicNode.updateNode(TT_BasicNode.java:313)
at com.sun.javatest.exec.TT_BasicNode.getChildCount(TT_BasicNode.java:88)
at com.sun.javatest.exec.TestTreeModel.getChildCount(TestTreeModel.java:132)
at javax.swing.tree.FixedHeightLayoutCache$FHTreeStateNode.expand(FixedHeightLayoutCache.java:1135)
at javax.swing.tree.FixedHeightLayoutCache.ensurePathIsExpanded(FixedHeightLayoutCache.java:645)
at javax.swing.tree.FixedHeightLayoutCache.setExpandedState(FixedHeightLayoutCache.java:282)

Jonathan Gibbons

Quite apart from the exception itself, reading the test suite on the AWT
event thread looks very suspect!

-- jon

jtharness@mobileandembedded.org wrote:
> There seems to be a defect. I opened a test suite with a single large JTD file. It produced this exception before it finished loading tests:
> Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException
> at com.sun.javatest.finder.BinaryTestFinder.readBinaryFile(BinaryTestFinder.java:332)
> at com.sun.javatest.finder.BinaryTestFinder.scan(BinaryTestFinder.java:192)
> at com.sun.javatest.TestFinder.read(TestFinder.java:439)
> at com.sun.javatest.TRT_TreeNode.processFile(TRT_TreeNode.java:1076)
> at com.sun.javatest.TRT_TreeNode.scanIfNeeded(TRT_TreeNode.java:639)
> at com.sun.javatest.TRT_TreeNode.getTreeNodes(TRT_TreeNode.java:169)
> at com.sun.javatest.exec.TT_BasicNode.updateNode0(TT_BasicNode.java:319)
> at com.sun.javatest.exec.TT_BasicNode.updateNode(TT_BasicNode.java:313)
> at com.sun.javatest.exec.TT_BasicNode.getChildCount(TT_BasicNode.java:88)
> at com.sun.javatest.exec.TestTreeModel.getChildCount(TestTreeModel.java:132)
> at javax.swing.tree.FixedHeightLayoutCache$FHTreeStateNode.expand(FixedHeightLayoutCache.java:1135)
> at javax.swing.tree.FixedHeightLayoutCache.ensurePathIsExpanded(FixedHeightLayoutCache.java:645)
> at javax.swing.tree.FixedHeightLayoutCache.setExpandedState(FixedHeightLayoutCache.java:282)
> [Message sent by forum member 'bkurotsu' (bkurotsu)]
>
> http://forums.java.net/jive/thread.jspa?messageID=328790
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: interest-unsubscribe@jtharness.dev.java.net
> For additional commands, e-mail: interest-help@jtharness.dev.java.net
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: interest-unsubscribe@jtharness.dev.java.net
For additional commands, e-mail: interest-help@jtharness.dev.java.net

bkurotsu
Offline
Joined: 2004-12-13

True true.

Fix reverted. The error wasn't fatal, but the harness continued to load tests at the blazing rate of about 2 tests per second.

I tried to fix it for a few minutes, but had no success. I suspect broken logic about when to open/close/reload the binary file - causing excessive file open/close.

Brian

sergey_borodin
Offline
Joined: 2006-10-20

Fix seems to be safe, I'm agree.
Please, go ahead and integrate.

Thanks,
Sergeey Borodin