ft: report a failed extra-file hashing instead of dropping it - #369
Open
jolavillette wants to merge 1 commit into
Open
ft: report a failed extra-file hashing instead of dropping it#369jolavillette wants to merge 1 commit into
jolavillette wants to merge 1 commit into
Conversation
ftExtraList::hashAFile() only had a success path: when RsDirUtil::hashFile() returned false the file was simply forgotten -- not recorded, no event, not even a log line. Every caller is then left waiting for a hash that will never come, since the file never enters mHashedList and ExtraFileStatus() keeps answering "not ready" forever. The web UI attach dialog turns that into a spinner that cannot be dismissed and one /rsFiles/ExtraFileStatus per second for the rest of the session. hashExtraFile() already refuses missing paths and directories, so reaching the failure means the file was there and still could not be read: permissions, a share unmounted or a file renamed between the check and this thread, an I/O error, or the hashing thread being stopped. Adds RsSharedDirectoriesEventCode::EXTRA_LIST_FILE_HASH_FAILED, carrying the path in the mFilePath field the event already has -- whose comment on mFileHash, "null if error occurred", shows the case was meant to exist. The success event now fills mFilePath and mFileHash too, so a listener can tell which file it is about instead of re-reading the whole extra list. While here, the event is posted outside extMutex rather than under it: posting while holding a service lock invites a deadlock through a handler.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ftExtraList::hashAFile()only had a success path. WhenRsDirUtil::hashFile()returned false the file was simply forgotten: not recorded, no event, not even a log line.Every caller is then left waiting for a hash that will never come. The file never enters
mHashedList, soExtraFileStatus()keeps answering "not ready" for ever — the web UI attach dialog turns that into a spinner it cannot dismiss and one/rsFiles/ExtraFileStatusper second for the rest of the session.hashExtraFile()has already refused the missing paths and the directories at submission, so reaching the failure means the file was there and still could not be read: permissions, a share unmounted or a file renamed between the check and this thread picking the job up, an I/O error, or the hashing thread being stopped.This adds:
RsSharedDirectoriesEventCode::EXTRA_LIST_FILE_HASH_FAILED, carrying the path in themFilePathfield the event already has — the comment onmFileHash, "null if error occurred", shows the case was meant to exist;RsErr()naming the file, so the failure is at least visible in the log;mFilePathandmFileHashfilled on the success event too, so a listener can tell which file it is about instead of re-reading the whole extra list.While here, the event is posted outside
extMutexrather than under it: posting while holding a service lock invites a deadlock through a handler.Adding an enum value is safe for the existing consumers — the three
switches overRsSharedDirectoriesEventCodein retroshare-gui (hashingstatus.cpp,RemoteDirModel.cpp,SharedFilesDialog.cpp) all have adefault.