downloader: manual .lock remove may lead to race and creation of data files without .torrent #9782
Conversation
AskAlexSharov
changed the title
| if !tf.exists(name) && !prohibited { | ||
| err = tf.create(name, res) | ||
| if err != nil { | ||
| return nil, false, false, err |
There was a problem hiding this comment.
created is always false.
Does it makes sense to just check lock file exists on the start instead of checking it per file? Like, we started, downloads either prohibited or not, that's it. Could be changed only with restart.
There was a problem hiding this comment.
“ instead of checking it per file? ” - this feature is just introduced by Giulio - to prevent downloading 1 type of file (he working on nee type of file): #9766
Don’t know: maybe it was doable by creating special branch in “erigon-snapshots” repo (maybe not).
|
AskAlexSharov
changed the title
| } | ||
| func (tf *TorrentFiles) create(torrentFilePath string, res []byte) error { | ||
|
|
||
| func (tf *TorrentFiles) CreateIfNotProhibited(name string, res []byte) (ts *torrent.TorrentSpec, prohibited, created bool, err error) { |
There was a problem hiding this comment.
prohibited and created results are not used anywhere - are they going to be used in a future PR? if not then should we remove them from the func signature?
also looks like created is always returned as false - seems like we are missing a statement to set created to true and return that in the case of a file getting created
such pattern is always race - even if
prohibited/exists()/create()are atomic inside.to eliminate race - need move towards analog of
CompareAndSwap-CreateIfNotProhibitedwhich in 1 mutex lock will: check_lock, existence, create.