On 04/04/17 09:37, Chris Dollin wrote:
I ran a simplified version of the test code, with no
sort but with a limit, and could not produce warnings.
So I'd not do anything to change TopN unless/until we
have more warnings to work with.
Let's fix it anyway. Trust the code analysis.
Have we stabilised enough that I turn this into a JIRA/PR with
a unit test?
Yes.
Andy
Chris
On 3 April 2017 at 15:34, Chris Dollin <[email protected]> wrote:
Argh, yes, all occurences of "open()" are supposed
to be "close()".
| Have you seen warnings in the TopN case?
I haven't got an isolated TopN test case but it has a similar
pattern of keeping references to the source iterator but
not closing via them.
I can construct an example and see what happens.
Chris
On 3 April 2017 at 11:15, Andy Seaborne <[email protected]> wrote:
On 31/03/17 16:40, Chris Dollin wrote:
Hi Andy
Yes, the cancel thread doesn't call open(), I understand
that.
is that "close()" not "open()"?
The warnings are intermittent because the iterator
in question (the "source" interator of QueryIterSort)
is only left open if the cancel request is noticed while
the sort databag is filling up.
QueryIterSort never explicitly closes the source
iterator. But if the sort databag pulls all of the
source bindings, the source iterator self-closes.
So there's a window while the bag is filling when
a detected cancellation will leave the source
iterator open.
A fix is to ensure that the source iterator
is closed when QueryIterSort closes:
@Override public void closeIterator() {
and also:
this.db.close();
c.f. requestCancel
embeddedIterator.close();
super.closeIterator();
}
"embeddedIterator" is the source iterator.
SortedDataBag: add close() to cancel:
public void cancel() {
comparator.cancel();
close();
}
This fix eliminates the presenting problem and
the tests continue to pass /except/ that
CallBackIterator's close() method needs to
be made a no-op rather than failing, since
that open() is now actually called.
close()?
[It looks like QueryIterTopN has the same problem]
It would do no harm to have the same close processing in QueryIterTopN
though it does not use data bags because it is an unconditional single pass
over the data.
Have you seen warnings in the TopN case?
Andy
Chris
On 20 March 2017 at 15:48, Chris Dollin <[email protected]>
wrote:
On 17 March 2017 at 17:08, Andy Seaborne <[email protected]> wrote:
SortedDataBag.cancel does not call close.
You're quite right, am having another look at the problem.
Chris
--
"What I don't understand is this ..." Trevor Chaplin, /The Beiderbeck
Affair/
Epimorphics Ltd, http://www.epimorphics.com
Registered address: Court Lodge, 105 High Street, Portishead, Bristol BS20
6PT
Epimorphics Ltd. is a limited company registered in England (number
7016688)
