Skip to content

Improve Linux event-handling robustness and allow provider-generated events#1189

Merged
chrisd8088 merged 4 commits intomicrosoft:features/linuxprototypefrom
github:linux-projfs-event-fixes
May 24, 2019
Merged

Improve Linux event-handling robustness and allow provider-generated events#1189
chrisd8088 merged 4 commits intomicrosoft:features/linuxprototypefrom
github:linux-projfs-event-fixes

Conversation

@chrisd8088
Copy link
Contributor

@chrisd8088 chrisd8088 commented May 22, 2019

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.

Separately, 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 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.

/cc @kivikakk

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.
@chrisd8088 chrisd8088 force-pushed the linux-projfs-event-fixes branch from 3b2dcee to bf165e9 Compare May 24, 2019 18:58
@chrisd8088
Copy link
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! 👍)

Copy link
Contributor

@derrickstolee derrickstolee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@chrisd8088 chrisd8088 merged commit 3babd8b into microsoft:features/linuxprototype May 24, 2019
@chrisd8088 chrisd8088 deleted the linux-projfs-event-fixes branch May 24, 2019 20:07
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants