Skip to content

ft: report a failed extra-file hashing instead of dropping it - #369

Open
jolavillette wants to merge 1 commit into
RetroShare:masterfrom
jolavillette:fix/extra-file-hash-failure-silent
Open

ft: report a failed extra-file hashing instead of dropping it#369
jolavillette wants to merge 1 commit into
RetroShare:masterfrom
jolavillette:fix/extra-file-hash-failure-silent

Conversation

@jolavillette

Copy link
Copy Markdown
Contributor

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. The file never enters mHashedList, so ExtraFileStatus() keeps answering "not ready" for ever — the web UI attach dialog turns that into a spinner it cannot dismiss and one /rsFiles/ExtraFileStatus per 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 the mFilePath field the event already has — the comment on mFileHash, "null if error occurred", shows the case was meant to exist;
  • an RsErr() naming the file, so the failure is at least visible in the log;
  • mFilePath and mFileHash filled 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 extMutex rather 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 over RsSharedDirectoriesEventCode in retroshare-gui (hashingstatus.cpp, RemoteDirModel.cpp, SharedFilesDialog.cpp) all have a default.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant