mafintosh / torrent-stream

The low level streaming torrent engine that peerflix uses
MIT License
1.94k stars 227 forks source link

fix deselect removing the wrong selection #125

Closed bsuh closed 9 years ago

bsuh commented 9 years ago

When calling file.createReadStream on the same file multiple times with the same range, one filestream can get stuck not reading any info.

This is because engine.deselect doesn't consider notify functions when differentiating selections.

When filestream B closes, engine.deselect will remove a selection notifying filestream A, since their from, to, priority are the same. Then filestream A never gets notified when new pieces are ready to be read.

asapach commented 9 years ago

Looks good.

mafintosh commented 9 years ago

we could also just have engine.select return a function that would deselect that range. that seem seem less hackish to me.

var deselect = engine.select(...)
deselect() // deselects that range you selected above

@asapach @bsuh what do you think?

asapach commented 9 years ago

Yes, it does look cleaner. My concern is that it might not address the problem extensively: when you call file.select()/file.deselect(), file.createReadStream() or engine.select()/engine.deselect() directly, you want to make sure that your selection does not interfere with the others. @bsuh's approach seems to satisfy most of the use cases, IMHO.

mafintosh commented 9 years ago

@asapach good point

mafintosh commented 9 years ago

Released in 0.20.0!