Improve Linux event-handling robustness and allow provider-generated events#1189
Merged
chrisd8088 merged 4 commits intomicrosoft:features/linuxprototypefrom May 24, 2019
Merged
Conversation
kivikakk
reviewed
May 23, 2019
97189a1 to
d902623
Compare
31 tasks
kivikakk
approved these changes
May 24, 2019
d902623 to
3b2dcee
Compare
1445f95 to
cba0e18
Compare
When the provider process generates a permission request event (e.g., by deleting a file in order to conform the working directory during a "git checkout"), this event is then delivered back to the provider itself for validation. We want to always allow those provider-generated permission- request events to succeed, so we return PROJFS_ALLOW for them, which we were previously failing to do.
We may fail to read the /proc/<pid>/cmdline file on Linux for a variety of reasons, not solely that it does not exist. In all cases, we should catch the exception and simply return an empty string; otherwise, the unhandled exception will unexpectedly halt the provider process.
We have been sending the source path, not the target path, in OnFileRenamed() events on Linux, but it's the latter which is expected by both the MirrorProvider and GVFS provider code.
3b2dcee to
bf165e9
Compare
Contributor
Author
|
I think this one is ready for a review, @derrickstolee -- thanks very much, and have a good weekend! (And thanks for fixing those CI issues! 👍) |
chrisd8088
added a commit
that referenced
this pull request
Jul 20, 2019
Improve and correct Linux event handling and allow provider-generated events
When the provider process generates a permission request event (e.g., by deleting a file in order to conform the working directory during a git checkout), this event is then delivered back to the provider itself for validation. We want to always allow those provider-generated permission-request events to succeed, so we return PROJFS_ALLOW for them, which we were previously failing to do.
Secondly, we may fail to read the /proc/{pid}/cmdline file on Linux for a variety of reasons, not solely that it does not exist. In all cases, we should catch the exception and simply return an empty string; otherwise, the unhandled exception will unexpectedly halt the provider process.
Thirdly, we've been sending the incorrect path on OnFileRenamed and OnHardLinkCreate events; it is expected to be the destination (a.k.a. target) path.
And lastly, we can remove a chunk of unnecessary overhead and code when handling non-projection events, because our OnNotifyOperation() method (which was derived from the Mac version), doesn't need most of its arguments. Neither does the Mac one, but its signature has to match the one implemented in its corresponding C++ library.
chrisd8088
added a commit
that referenced
this pull request
Jul 26, 2019
Improve and correct Linux event handling and allow provider-generated events
When the provider process generates a permission request event (e.g., by deleting a file in order to conform the working directory during a git checkout), this event is then delivered back to the provider itself for validation. We want to always allow those provider-generated permission-request events to succeed, so we return PROJFS_ALLOW for them, which we were previously failing to do.
Secondly, we may fail to read the /proc/{pid}/cmdline file on Linux for a variety of reasons, not solely that it does not exist. In all cases, we should catch the exception and simply return an empty string; otherwise, the unhandled exception will unexpectedly halt the provider process.
Thirdly, we've been sending the incorrect path on OnFileRenamed and OnHardLinkCreate events; it is expected to be the destination (a.k.a. target) path.
And lastly, we can remove a chunk of unnecessary overhead and code when handling non-projection events, because our OnNotifyOperation() method (which was derived from the Mac version), doesn't need most of its arguments. Neither does the Mac one, but its signature has to match the one implemented in its corresponding C++ library.
chrisd8088
added a commit
that referenced
this pull request
Jul 28, 2019
Improve and correct Linux event handling and allow provider-generated events
When the provider process generates a permission request event (e.g., by deleting a file in order to conform the working directory during a git checkout), this event is then delivered back to the provider itself for validation. We want to always allow those provider-generated permission-request events to succeed, so we return PROJFS_ALLOW for them, which we were previously failing to do.
Secondly, we may fail to read the /proc/{pid}/cmdline file on Linux for a variety of reasons, not solely that it does not exist. In all cases, we should catch the exception and simply return an empty string; otherwise, the unhandled exception will unexpectedly halt the provider process.
Thirdly, we've been sending the incorrect path on OnFileRenamed and OnHardLinkCreate events; it is expected to be the destination (a.k.a. target) path.
And lastly, we can remove a chunk of unnecessary overhead and code when handling non-projection events, because our OnNotifyOperation() method (which was derived from the Mac version), doesn't need most of its arguments. Neither does the Mac one, but its signature has to match the one implemented in its corresponding C++ library.
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.
When the provider process generates a permission request event (e.g., by deleting a file in order to conform the working directory during a
git checkout), this event is then delivered back to the provider itself for validation.We want to always allow those provider-generated permission-request events to succeed, so we return
PROJFS_ALLOWfor them, which we were previously failing to do.Separately, we may fail to read the
/proc/{pid}/cmdlinefile on Linux for a variety of reasons, not solely that it does not exist.In all cases, we should catch the exception and simply return an empty string; otherwise, the unhandled exception will unexpectedly halt the provider process.
Thirdly, we've been sending the incorrect path on
OnFileRenamedevents; it is expected to be the destination (a.k.a. target) path.And lastly, we can remove a chunk of unnecessary overhead and code when handling non-projection events, because our
OnNotifyOperation()method (which was derived from the Mac version), doesn't need most of its arguments. Neither does the Mac one, but its signature has to match the one implemented in its corresponding C++ library./cc @kivikakk