1513865 - [socket-process] nsIPrefService.resetPrefs does not work

Status: RESOLVED WORKSFORME

(Core :: Networking, defect, P2)

Description

[Dragana Damjanovic [:dragana]]

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).

Comment 1

[Dragana Damjanovic [:dragana]]

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.

Comment 2

[Dragana Damjanovic [:dragana]]

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

Comment 3

[Kris Maglione [:kmag]]

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.

Comment 4

[Dragana Damjanovic [:dragana]]

(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!!!

Comment 5

[Nhi Nguyen (:nhi)]

removed blocking on bug 1509472 based on my discussion with Dragana

Comment 6

[Dragana Damjanovic [:dragana]]

Junior, can you take this one?
(I will forward you some pointers per email)

Comment 7

[Junior [inactive]]

Yes.

Comment 8

[Ron Manning]

Since this work is no longer a part of Project Fission, can we remove it from the meta bug?
https://bugzilla.mozilla.org/show_bug.cgi?id=1451850

Comment 9

[Junior [inactive]]

bug 1546355 eases this work

Comment 10

[Junior [inactive]]

Not sure why [1] can't get the pref set by test

https://searchfox.org/mozilla-central/rev/227f5329f75bd8b16c6b146a7414598a420260cb/netwerk/base/nsIOService.cpp#408

Comment 11

[Junior [inactive]]

Well, bug 1555792 disabled socket process for xpcshell tests
https://searchfox.org/mozilla-central/rev/9ae20497229225cb3fa729a787791f424ff8087b/netwerk/base/nsIOService.cpp#410

Comment 12

[Junior [inactive]]

Comment 11