Open
Bug 1350438
Opened 8 years ago
Updated 2 years ago
OS.Constants.Path.tmpDir returns a stale value
Categories
(Toolkit :: IOUtils and PathUtils, enhancement)
Toolkit
IOUtils and PathUtils
Tracking
()
NEW
People
(Reporter: mccr8, Unassigned)
References
Details
OSFileConstantsService gets the value of TmpD from the directory service. However, the value of TmpD in a sandboxed content process does not stay the same.
Initially, TmpD has the same value as the parent process, but eventually ContentProcess::Init() calls SetUpSandboxEnvironment() and changes it to a special content process temp directory. This happens fairly early in startup, but not before we can start running JS. If, say, telemetry eagerly loads osfile.jsm then we end up with a stale value for OS.Constants.Path.tmpDir. (See bug 1349389 for more context.)
My patch in bug 1349389 keeps us from loading osfile.jsm until later, but that feels like a shaky invariant. At a minimum we should assert that OSFileConstantsService is started after SetUpSandboxEnvironment(). A slicker approach would be to somehow update the value of OS.Constants.Path.tmpDir but I don't know how much of a pain that would be.
Reporter | ||
Updated•8 years ago
|
> A slicker approach would be to somehow update the value of OS.Constants.Path.tmpDir but I don't know how much of a pain that would be.
This would be ideal, but difficult due to threads: OS.Constants can be accessed by any thread, while the directory service can only be accessed from the main thread, so we need to pre-read the values.
Comment 2•8 years ago
|
||
(In reply to David Teller [:Yoric] (please use "needinfo") from comment #1)
> > A slicker approach would be to somehow update the value of OS.Constants.Path.tmpDir but I don't know how much of a pain that would be.
>
> This would be ideal, but difficult due to threads: OS.Constants can be
> accessed by any thread, while the directory service can only be accessed
> from the main thread, so we need to pre-read the values.
The "feature" of OS File jsms loading automatically (greedily) fetching various paths out of the directory service is problematic, IMO. It bit me in bug 1344759, see bug 1348103 (where it's now biting code coverage builds, still, because those import osfile themselves).
When you say "OS.Constants can be accessed by any thread" - can you clarify? Who does off-main-thread access here? I would assume those would end up using their own compartments and instances, because they're on a different thread...
How hard would it be to make the directory service non-mainthread-only? It feels like an odd restriction anyway - the paths should be available everywhere.
Flags: needinfo?(dteller)
> When you say "OS.Constants can be accessed by any thread" - can you clarify? Who does off-main-thread access here? I would assume those would end up using their own compartments and instances, because they're on a different thread...
ChromeWorker threads have access to `OS.Constants`. I'm not sure I understand your question.
> How hard would it be to make the directory service non-mainthread-only? It feels like an odd restriction anyway - the paths should be available everywhere.
This might become possible once we get rid of XPCOM in add-ons. For the moment, it's impossible, as add-ons can extend it from JS code and this would end up crashing messily. Once that prerequisite is satisfied, exposing it to OS.Constants will still require some work, but this should be feasible.
Flags: needinfo?(dteller)
Comment 4•3 years ago
|
||
OSFile is being replaced with IOUtils and PathUtils. However, since we are doing some path caching in PathUtils, we should double check that we are only initializing its tempdir members after SetUpSandboxEnvironment
Updated•2 years ago
|
Severity: normal → S3
Updated•2 years ago
|
Component: OS.File → IOUtils and PathUtils
You need to log in
before you can comment on or make changes to this bug.
Description
•