tantivy icon indicating copy to clipboard operation
tantivy copied to clipboard

Protect against infinite loop in skip reader seek

Open fmassot opened this issue 4 years ago • 2 comments

Is your feature request related to a problem? Please describe. When using (badly) the scorer API and especially the seek method, I ended up in some infinite loop.

It comes from this little piece for code in the SkipReader:

pub fn seek(&mut self, target: DocId) -> bool {
    if self.last_doc_in_block() >= target {
        return false;
    }
    loop {
        self.advance();
        if self.last_doc_in_block() >= target {
            return true;
        }
    }
}

Unfortunately for me, my function was doing a seek on a doc_id + 1 without checking the value was TERMINATED... I understand that this is a low-level API and that the programmer should be careful. What I suggest is to add a debug_assert in the seek of the SkipReader so that we have the feedback immediately.

fmassot avatar Oct 30 '21 17:10 fmassot

debug_assert!(target <= TERMINATED) sounds like a good idead.

I am not sure whether debug_assert!(target < TARGET) would make sense though. I think we rely on target= TERMINATED being legal in the intersection code.

fulmicoton avatar Oct 31 '21 04:10 fulmicoton

Yes the first assert would be good, there is no infinite loop on target = TERMINATED. Let me make a nice PR :)

fmassot avatar Oct 31 '21 10:10 fmassot