Github user ehedgehog commented on the issue:
https://github.com/apache/jena/pull/236
I realised that there were no test cases for QueryIterTopN, which has
a failure mode like TestQuerySort. I added three tests to QueryIterSort
(because I reused the CallbackIterator from TestQueryIterSort and
extracting that to a new file would make the diffs more complicated)
in the same style as the tests for QueryIterSort.close():
================================================
@Test public void testTopNCloseClosesSource() {
long numItems = 3;
boolean distinct = false;
Context context = new Context() ;
ExecutionContext ec = new ExecutionContext(context, (Graph)
null, (DatasetGraph) null, (OpExecutorFactory) null);
QueryIterTopN tn = new QueryIterTopN(iterator, comparator,
numItems, distinct, ec);
tn.close();
assertTrue(iterator.isClosed());
}
@Test public void testTopNExhaustionClosesSource() {
Callback nullCB = new Callback() { @Override public void call()
{} };
iterator.setCallback(nullCB);
long numItems = 3;
boolean distinct = false;
Context context = new Context() ;
ExecutionContext ec = new ExecutionContext(context, (Graph)
null, (DatasetGraph) null, (OpExecutorFactory) null);
QueryIterTopN tn = new QueryIterTopN(iterator, comparator,
numItems, distinct, ec);
while (tn.hasNext()) tn.next();
assertTrue(iterator.isClosed());
}
@Test public void testTopNCancelClosesSourceIterator() {
long numItems = 3;
boolean distinct = false;
Context context = new Context() ;
ExecutionContext ec = new ExecutionContext(context, (Graph)
null, (DatasetGraph) null, (OpExecutorFactory) null);
QueryIterTopN tn = new QueryIterTopN(iterator, comparator,
numItems, distinct, ec);
try {
while (tn.hasNext()) tn.next();
fail("QueryCancelledException not caught");
} catch (QueryCancelledException q) {
assertTrue("cancellation did not close source
iterator", iterator.isClosed());
}
}
==========================================
It turns out that the third of these tests fails because an exception
thrown during QueryIterTopN's initializeIterator does not close
the source (or any other) iterator. So I wrapped the body of the
initializeIterator in a try-catch-QueryCancelledException and closed
both the initializeIterator nad the QueryIterTopN.
===============================================
private Iterator<Binding> sortTopN(final QueryIterator qIter, final
Comparator<Binding> comparator) {
return new IteratorDelayedInitialization<Binding>() {
@Override
protected Iterator<Binding> initializeIterator() {
try {
while ( qIter.hasNext() ) {
Binding binding = qIter.next() ;
if ( heap.size() < limit )
add(binding) ;
else {
Binding currentMaxLeastN = heap.peek() ;
if ( comparator.compare(binding,
currentMaxLeastN) < 0 )
add(binding) ;
}
}
qIter.close() ;
Binding[] y = heap.toArray(new Binding[]{}) ;
heap = null ;
Arrays.sort(y, comparator) ;
return asList(y).iterator() ;
} catch (QueryCancelledException e) {
QueryIterTopN.this.close();
this.close();
throw e;
}
}
} ;
}
================================================
The new tests now pass.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
