Page MenuHomePhabricator
Differential D11501

[keyserver] Don't block innerPerformAuth return on sleep
ClosedPublic
Actions

Authored by ashoat on Mar 31 2024, 7:45 PM.
Tags
None
Referenced Files
F3356814: D11501.diff
Sat, Nov 23, 8:51 PM
Unknown Object (File)
Tue, Nov 12, 2:39 PM
Unknown Object (File)
Tue, Nov 12, 2:39 PM
Unknown Object (File)
Tue, Nov 12, 12:59 PM
Unknown Object (File)
Tue, Nov 12, 9:19 AM
Unknown Object (File)
Fri, Nov 1, 10:34 PM
Unknown Object (File)
Oct 11 2024, 9:16 AM
Unknown Object (File)
Oct 11 2024, 9:16 AM
View All 45 Files
Subscribers
None

Details

Reviewers
tomek
inka
Commits
rCOMMeb1952d00f4b: [keyserver] Don't block innerPerformAuth return on sleep
Summary

In an earlier version of this stack, in innerPerformRecovery I would fall back on legacy login, and without this change the fallback would have to wait on this sleep.

This didn't used to matter before because nothing actually waits for innerPerformAuth to complete. And since I eventually came to the conclusion that the fallback was a bad idea (we'd be sending user passwords to keyservers), it actually still doesn't matter.

I decided to put this diff up anyways because I think it's probably better for us to avoid blocking the return of innerPerformAuth on this sleep.

Depends on D11500

Test Plan

Confirm in the earlier version of the stack that when falling back on legacy login, there was no delay due to this sleep

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

ashoat created this revision.Mar 31 2024, 7:45 PM
ashoat added a child revision: D11502: [lib] Make invalidSessionRecovery work with all recoveryActionSources handled by KeyserverConnectionHandler.
Harbormaster completed remote builds in B27881: Diff 38614.Mar 31 2024, 8:03 PM
ashoat requested review of this revision.Mar 31 2024, 8:03 PM
tomek accepted this revision.Apr 2 2024, 2:47 AM
tomek added inline comments.
lib/components/keyserver-connection-handler.js
182–185 ↗(On Diff #38614)

It seems surprising to me that this sleep was blocking the return from innerPerformAuth - this code is part of a promise created on line 114, so it should immediately return. Maybe the reason is to avoid waiting for sleep to complete when awaiting the result of calling innerPerformAuth?

This revision is now accepted and ready to land.Apr 2 2024, 2:47 AM
ashoat added inline comments.Apr 2 2024, 6:48 AM
lib/components/keyserver-connection-handler.js
182–185 ↗(On Diff #38614)

Yes, it's about a callsite where we await the result of calling innerPerformAuth. I've highlighted it below

270–275 ↗(On Diff #38614)

The issue is that this promise will not resolve until the sleep concludes

Closed by commit rCOMMeb1952d00f4b: [keyserver] Don't block innerPerformAuth return on sleep (authored by ashoat). · Explain WhyApr 2 2024, 7:28 AM
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMMeb1952d00f4b: [keyserver] Don't block innerPerformAuth return on sleep.

Revision Contents
Changeset List

PathSize
lib/
components/
6 lines

Diff 38672

View Options

lib/components/keyserver-connection-handler.js

Loading...