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

getSnapshotIdFromPath() shouldn't assume parent and child process have same TmpD

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Developer Tools: Memory
RESOLVED FIXED
7 months ago
7 months ago

People

(Reporter: mccr8, Assigned: mccr8)

Tracking

unspecified
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

7 months ago
See bug 1349389 for details.

But anyways, what happens is the parent process creates a file in the temp directory, sends the file name to the child process which does this: 

exports.getSnapshotIdFromPath = function (path) {
  return path.slice(OS.Constants.Path.tmpDir.length + 1,
                    path.length - ".fxsnapshot".length);
};

With the new fancy sandboxing work that is going on, the temp dir in the parent and child are not the same. The child process temp dir has some extra "Temp-{UUID}" stuff added on the end. Therefore, getSnapshotIdFromPath will return an empty string if OS.Constants.Path.tmpDir returns the correct value.

This only happens to incidentally work right now due to a bug in OS.Constants.Path snapshotting the value of tmpDir early in startup, when we eagerly load osfile.jsm from telemetry startup. (I'm going to file a separate bug on that.)

I think the right fix is to compute the snapshot ID in the parent, and send it in the message instead of the path.
(Assignee)

Updated

7 months ago
Assignee: nobody → continuation
Comment hidden (mozreview-request)
(Assignee)

Comment 2

7 months ago
I need to remember to get a DOM peer review for the .webidl change once fitzgen reviews this (I added a chrome-only method that is just a variation of another method, so I can't imagine it will need more than a rubber stamp DOM review).

Comment 3

7 months ago
mozreview-review
Comment on attachment 8851347 [details]
Bug 1350435 - Compute snapshot ID in the parent process.

https://reviewboard.mozilla.org/r/123662/#review126370

Looks great! Thanks :mccr8 :)

Comment 4

7 months ago
mozreview-review
Comment on attachment 8851347 [details]
Bug 1350435 - Compute snapshot ID in the parent process.

https://reviewboard.mozilla.org/r/123662/#review126372
Attachment #8851347 - Flags: review?(nfitzgerald) → review+
(Assignee)

Updated

7 months ago
Attachment #8851347 - Flags: review?(bugs)
(Assignee)

Comment 5

7 months ago
Olli, could you look at the .webidl change? Thanks.

Comment 6

7 months ago
mozreview-review
Comment on attachment 8851347 [details]
Bug 1350435 - Compute snapshot ID in the parent process.

https://reviewboard.mozilla.org/r/123662/#review126380

r+ for the .webidl
Attachment #8851347 - Flags: review?(bugs) → review+

Comment 7

7 months ago
Pushed by amccreight@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/951e3cdf5b32
Compute snapshot ID in the parent process. r=fitzgen,smaug

Comment 8

7 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/951e3cdf5b32
Status: NEW → RESOLVED
Last Resolved: 7 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
You need to log in before you can comment on or make changes to this bug.