[socket-process] nsIPrefService.resetPrefs does not work

ASSIGNED
Assigned to

Status

()

defect
P2
normal
ASSIGNED
4 months ago
6 days ago

People

(Reporter: dragana, Assigned: dragana)

Tracking

(Blocks 2 bugs)

63 Branch
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Fission Milestone:M2)

Details

(Whiteboard: [necko-triaged])

(Assignee)

Description

4 months ago
resetPref is only use on some xpcshell tests, therefore it was not made to work with  Preferences::EnsureSnapshot (before the socket process work Preferences::EnsureSnapshot is used only for the content process which do not run during xpcshell tests).
(Assignee)

Comment 1

4 months ago
Can you give me a short explanation what needs to be done here to make resetPrefs work with the Socket process that will use gSharedMap.
Flags: needinfo?(kmaglione+bmo)
(Assignee)

Comment 2

3 months ago

we can also turn off the socket process for this tests.

Sorry for the delay.

(In reply to Dragana Damjanovic [:dragana] from comment #1)

Can you give me a short explanation what needs to be done here to make
resetPrefs work with the Socket process that will use gSharedMap.

This is much easier said than done.

So, to be clear, this would not have correctly after child process startup even before shared preferences existed. It would have caused the preferences in the parent process to be reset, but not child processes, which means that parent and child processes would quickly become out of sync.

With shared preferences, the situation becomes a bit harder. There are some ways we could handle this, with varying degrees of brokenness, but all of them require leaking the shared map memory, since any string references which come from it are treated as permanent literals, and are not refcounted.

(In reply to Dragana Damjanovic [:dragana] from comment #2)

we can also turn off the socket process for this tests.

That's probably the best option. Any attempt to support resetting the pref service in a multi-process world is going to require a nontrivial amount of intricate code. And this function is only used by a few highly specific pref service unit tests, and should never be used elsewhere. Since those tests don't depend on the socket process at all (and don't even use the network), there's really no reason we should spend effort trying to make the socket process work with them.

Flags: needinfo?(kmaglione+bmo)
(Assignee)

Comment 4

3 months ago

(In reply to Kris Maglione [:kmag] from comment #3)

Sorry for the delay.

(In reply to Dragana Damjanovic [:dragana] from comment #1)

Can you give me a short explanation what needs to be done here to make
resetPrefs work with the Socket process that will use gSharedMap.

This is much easier said than done.

So, to be clear, this would not have correctly after child process startup even before shared preferences existed. It would have caused the preferences in the parent process to be reset, but not child processes, which means that parent and child processes would quickly become out of sync.

With shared preferences, the situation becomes a bit harder. There are some ways we could handle this, with varying degrees of brokenness, but all of them require leaking the shared map memory, since any string references which come from it are treated as permanent literals, and are not refcounted.

(In reply to Dragana Damjanovic [:dragana] from comment #2)

we can also turn off the socket process for this tests.

That's probably the best option. Any attempt to support resetting the pref service in a multi-process world is going to require a nontrivial amount of intricate code. And this function is only used by a few highly specific pref service unit tests, and should never be used elsewhere. Since those tests don't depend on the socket process at all (and don't even use the network), there's really no reason we should spend effort trying to make the socket process work with them.

Thanks!!!

Updated

3 months ago
Fission Milestone: --- → M2
(Assignee)

Updated

3 months ago
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
You need to log in before you can comment on or make changes to this bug.