If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Fennec weave client downloads privkey hundreds of times instead of caching it

RESOLVED FIXED in 0.3

Status

Cloud Services
General
P1
critical
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: Jono Xia, Unassigned)

Tracking

unspecified
ARM
Maemo
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

9 years ago
A brief section from the very long logs I get when the Fennec weave client is syncing:

2009-02-26 11:25:14	PrivKeyManager	TRACE	Importing record: https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:14	Net.Resource	DEBUG	GET request for https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:14	Net.Resource	DEBUG	GET request successful (200)
2009-02-26 11:25:14	Store.BStore	TRACE	Updating {d8d675c8-e328-8d48-9092-706bb31c5b46}1 (213)
2009-02-26 11:25:14	Store.BStore	TRACE	Moving item (changing folder/index)
2009-02-26 11:25:14	PrivKeyManager	TRACE	Importing record: https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:14	Net.Resource	DEBUG	GET request for https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:14	Net.Resource	DEBUG	GET request successful (200)
2009-02-26 11:25:14	Store.BStore	TRACE	Updating {d8d675c8-e328-8d48-9092-706bb31c5b46}2 (214)
2009-02-26 11:25:14	Store.BStore	TRACE	Moving item (changing folder/index)
2009-02-26 11:25:14	PrivKeyManager	TRACE	Importing record: https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:14	Net.Resource	DEBUG	GET request for https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:14	Net.Resource	DEBUG	GET request successful (200)

...


2009-02-26 11:25:17	PrivKeyManager	TRACE	Importing record: https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:17	Net.Resource	DEBUG	GET request for https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:17	Net.Resource	DEBUG	GET request successful (200)
2009-02-26 11:25:18	PrivKeyManager	TRACE	Importing record: https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:18	Net.Resource	DEBUG	GET request for https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:18	Net.Resource	DEBUG	GET request successful (200)
2009-02-26 11:25:18	PrivKeyManager	TRACE	Importing record: https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:18	Net.Resource	DEBUG	GET request for https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:18	Net.Resource	DEBUG	GET request successful (200)
2009-02-26 11:25:18	PrivKeyManager	TRACE	Importing record: https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:18	Net.Resource	DEBUG	GET request for https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:18	Net.Resource	DEBUG	GET request successful (200)
2009-02-26 11:25:19	PrivKeyManager	TRACE	Importing record: https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey
2009-02-26 11:25:19	Net.Resource	DEBUG	GET request for https://sj-weave01.services.mozilla.com/0.3/user/jono/keys/privkey




It appears to be downloading the private key once *per* *record* that it's syncing in, which means hundreds of times.  I believe it's supposed to be caching this key, but apparently it's not.

This makes syncing on Fennec SLOOOOOOOWWWWWWW.

Comment 1

9 years ago
This is the code that gets the key:

http://hg.mozilla.org/labs/weave/file/tip/modules/base_records/crypto.js#l110


let pubkey = yield PubKeys.getDefaultKey(self.cb);
let privkey = yield PrivKeys.get(self.cb, pubkey.privateKeyUri);

PrivKeys is an instance of PrivkeyManager : PubkeyManager : RecordManager

http://hg.mozilla.org/labs/weave/file/tip/modules/base_records/keys.js#l225
http://hg.mozilla.org/labs/weave/file/tip/modules/base_records/keys.js#l169
http://hg.mozilla.org/labs/weave/file/tip/modules/base_records/wbo.js#l141

As you can see, get() only calls import() if this._records doesn't have a record for that url already in it, and import() saves the imported record into this._records.

Updated

9 years ago
Blocks: 468694
Severity: normal → critical
Priority: -- → P1
Target Milestone: -- → 0.3
(Reporter)

Comment 2

9 years ago
We were using the URI *objects* as keys in this._records, instead of the URI strings.  The object is different every time, even when the uri string is the same, therefore the cache was never hit.
(Reporter)

Comment 3

9 years ago
Fixed in http://hg.mozilla.org/labs/weave/rev/503670ad46c4
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED

Comment 4

9 years ago
Whoa!
How did this ever work?  It wasn't grabbing them over and over again in Firefox.

Comment 5

9 years ago
http://hg.mozilla.org/labs/weave/rev/503670ad46c4#l1.7
1.7 -  get keyData() this.payload.key_data,
1.8 +  get keyData() { return this.payload.key_data; },

Why that change to the getter?

js> let foo = { get bar() { return 5; }, get baz() 10 };
js> [foo.bar, foo.baz];
5,10

(I remember running into problems before where I couldn't do expression closures for getters, but it seems to be working now.)
(Reporter)

Comment 6

9 years ago
Ed Lee:  I added braces to that function so that I could put in some debugging output, when I was trying to fix the bug.  I removed the debugging output before pushing, but I didn't delete the braces.  There's no reason for them to be there.

Dan: I don't know how it worked in Firefox.  Maybe Firefox was always using the same URI object, so the cache was getting hit?  That doesn't make sense, though.  Do you want me to look into it?  A difference between the way the WBOs are used in Fennec vs. Firefox could be a clue that other things are wrong.

Comment 7

9 years ago
(In reply to comment #6)
> Do you want me to look into it?  A difference between the way the WBOs
> are used in Fennec vs. Firefox could be a clue that other things are wrong.

Yeah, but let's do that in a separate bug.  Filed bug 482289 for that.

Updated

8 years ago
Component: Weave → General
Product: Mozilla Labs → Weave
QA Contact: weave → general
You need to log in before you can comment on or make changes to this bug.