Mac: Fix missing files due to auth cache#270
Conversation
|
Confirmed at least one test failure without a fix. The rest of the tests pass both on my machine and on the server, but @wilbaker has confirmed that they fail on his machine, so we'll confirm the fix there once I push it up. https://dev.azure.com/mseng/VSOnline/_build/results?buildId=7238862&view=logs |
|
With the fix, the multithreaded read test now passes on the server: https://dev.azure.com/mseng/VSOnline/_build/results?buildId=7241672&view=logs |
|
I ran all of the CheckoutTests twice on my machine, and all passed each time. |
| Exception readException = null; | ||
|
|
||
| Thread[] threads = new Thread[32]; | ||
| Thread[] threads = new Thread[128]; |
There was a problem hiding this comment.
Why are we upping the number of threads here?
There was a problem hiding this comment.
Totally anecdotal - but I've just seen on my own machine that the higher number of threads makes any races more likely to get hit. And the more temporary/partial fixes I put in place in my other branch, the more threads I needed to reliably hit any remaining issues.
| Exception readException = null; | ||
|
|
||
| Thread[] threads = new Thread[32]; | ||
| Thread[] threads = new Thread[128]; |
nickgra
left a comment
There was a problem hiding this comment.
What a delightful PR to see after coming back from vacation :)
| vnode_put(virtualizationRootVNode); | ||
| } | ||
|
|
||
| if (rootIndex >= 0) |
There was a problem hiding this comment.
Should we reset the auth cache TTL back to the default upon virtualization root unmount?
There was a problem hiding this comment.
Yes we will certainly want to do that, but right now unmount doesn't call through to the kext at all so we're currently not doing any cleanup. I filed #284 to cover all of that.
The first commit re-enables all the tests that were hitting bugs with auth cache poison issues. The second commit fixes those issues by setting the auth cache TTL to 0.
With this fix, we no longer hit any missing file issues. Initial perf testing also demonstrates that there is very minimal overhead on non-virtualized paths, i.e. anything outside of a VFS for Git repo. For paths inside of a virtualization root, there is a bigger perf hit, but that is because our kext is now getting called on every kauth request, and any of our own perf issues are getting magnified. We will do more work to analyze that overhead and optimize it, but for now this fix makes our kext much more reliable.
Fixes #249