- Notifications
You must be signed in to change notification settings - Fork5
fix: improve wake & sleep handling#74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| } | ||
| overridefunc wake(){ | ||
| guard !reassertingelse{return} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
macOS does indeedwake a bunch during sleep to check the network. For us dogfooding, this means it might attempt to download a new dylib while the device sleeps, which the user won't be able to approve. In that case it'll get stuck inreasserting, in this case (and in any case, really) we don't want to try and start it up a second time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Got it. Would you also mind adding that as a code comment?
| globalXPCListenerDelegate.vpnXPCInterface.manager=nil | ||
| self.manager=nil | ||
| completionHandler() | ||
| } | ||
| self.manager=nil | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We wantself.manager being nil to be our code's perspective as to whether a stop operation has actually finished. Before, it was nil a little too early, though I don't think it was causing any issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Would you mind adding that as a code comment?
| guard manager==nilelse{ | ||
| logger.error("wake called with non-nil Manager") | ||
| return | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This was previously in thestart function called right after, but we should avoid resetting the network settings needlessly if we can.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
LGTM, small remarks about getting some comments in.
f970cfc intomainUh oh!
There was an error while loading.Please reload this page.
No description provided.