PSA: nsIURI implementations are now threadsafe

[Valentin Gosu]

Hello everyone,

I would like to announce that with the landing of bug 1447194, all nsIURI
implementations in Gecko are now threadsafe, as well as immutable. As a
consequence, you no longer have to clone a URI when you pass it around, as
it's guaranteed not to change, and now it's OK to release them off the main
thread.

If you need to change a nsIURI, you should use the nsIURIMutator interface
(in JavaScript - just call .mutate() on the URI) or the NS_MutateURI
<https://searchfox.org/mozilla-central/source/netwerk/test/gtest/TestURIMutator.cpp#22>
helper class (in C++).

More info here:
https://wiki.mozilla.org/Necko/nsIURI

If you find any bugs, make them block bug 922464 (OMT-nsIURI)

I'd like to thank everyone who helped review the patches, especially Honza
Bambas who reviewed most of my patches.

Cheers!

[Boris Zbarsky]

On 3/23/18 8:25 AM, Valentin Gosu wrote:
> I would like to announce that with the landing of bug 1447194, all nsIURI
> implementations in Gecko are now threadsafe, as well as immutable.

Valentin,

Thank you very much for doing this! Just the safety guarantees from URI
immutability are a huge deal.

Of course the threadsafety is nice too. ;)

-Boris

[Patrick McManus]

\o/ !!


On Friday, March 23, 2018, Valentin Gosu <valent...@gmail.com> wrote:

> Hello everyone,

>
> I would like to announce that with the landing of bug 1447194, all nsIURI

> implementations in Gecko are now threadsafe, as well as immutable. As a
> consequence, you no longer have to clone a URI when you pass it around, as
> it's guaranteed not to change, and now it's OK to release them off the main
> thread.
>
> If you need to change a nsIURI, you should use the nsIURIMutator interface
> (in JavaScript - just call .mutate() on the URI) or the NS_MutateURI
> <https://searchfox.org/mozilla-central/source/netwerk/test/gtest/
> TestURIMutator.cpp#22>
> helper class (in C++).
>
> More info here:
> https://wiki.mozilla.org/Necko/nsIURI
>
> If you find any bugs, make them block bug 922464 (OMT-nsIURI)
>
> I'd like to thank everyone who helped review the patches, especially Honza
> Bambas who reviewed most of my patches.
>
> Cheers!

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
>

[Valentin Gosu]

On 23 March 2018 at 21:06, Ben Kelly <bke...@mozilla.com> wrote:

> This is so great. Thank you!
>
> One question that comes to mind, though, is there any chance this could be
> uplifted to 60? As we start doing more OMT nsIURI stuff its going to
> become difficult to uplift code to 60ESR.
>

Certainly doable.
Most of the code landed in 60. Only 5 bugs related to this were landed in
61, 1442239 <https://bugzilla.mozilla.org/show_bug.cgi?id=1442239>, 1442242
<https://bugzilla.mozilla.org/show_bug.cgi?id=1442242>, 1447190
<https://bugzilla.mozilla.org/show_bug.cgi?id=1447190>, 1447194
<https://bugzilla.mozilla.org/show_bug.cgi?id=1447194>, 1447330
<https://bugzilla.mozilla.org/show_bug.cgi?id=1447330>. Still a fair amount
of code, but it might be worth it.


> On Fri, Mar 23, 2018 at 8:25 AM, Valentin Gosu <valent...@gmail.com>

[Ben Kelly]

Do we have any plan to be able to use NS_NewURI() off-main-thread? I
thought that was included here, but I see now that it is not. The initial
URL parse OMT is important for truly being able to remove all our "bounce
to the main thread for URL stuff" legacy code.

[Ben Kelly]

This is so great. Thank you!

One question that comes to mind, though, is there any chance this could be
uplifted to 60? As we start doing more OMT nsIURI stuff its going to
become difficult to uplift code to 60ESR.

[Valentin Gosu]

On 26 March 2018 at 16:53, Ben Kelly <bke...@mozilla.com> wrote:

> On Mon, Mar 26, 2018 at 10:47 AM, Valentin Gosu <valent...@gmail.com>
> wrote:
>
>> Yes, that is definitely something we want to fix, but not very
>> straightforward. We have quite a few URI implementations, and a bunch more
>> protocol handlers.
>>
>> https://github.com/valenting/gecko/wiki/Threadsafe-URIs-prog
>> ress#protocol-handler-implementations
>>
>
> I wonder if it would be worth adding something like:
>
> NS_ThreadsafeNewURI()
>
> That returns an error code if it detects that the URL does not match any
> of the threadsafe URL implementations. This would allow code to try
> threadsafe parsing first and only fall back to a main thread bounce if its
> an oddball URL. Right now we hardcode a bunch of http/https/nsStandardURL
> things in various places to accomplish this.
>
>

I think that's a pretty good strategy. It allows us to progressively move
things from nsIProtocolHandler.newURI to NS_ThreadsafeNewURI.

>
>>
>> On 26 March 2018 at 15:24, Ben Kelly <bke...@mozilla.com> wrote:
>>
>>> Do we have any plan to be able to use NS_NewURI() off-main-thread? I
>>> thought that was included here, but I see now that it is not. The initial
>>> URL parse OMT is important for truly being able to remove all our "bounce
>>> to the main thread for URL stuff" legacy code.
>>>

[Ben Kelly]

On Mon, Mar 26, 2018 at 10:47 AM, Valentin Gosu <valent...@gmail.com>
wrote:

> Yes, that is definitely something we want to fix, but not very
> straightforward. We have quite a few URI implementations, and a bunch more
> protocol handlers.
>
> https://github.com/valenting/gecko/wiki/Threadsafe-URIs-
> progress#protocol-handler-implementations
>

I wonder if it would be worth adding something like:

NS_ThreadsafeNewURI()

That returns an error code if it detects that the URL does not match any of
the threadsafe URL implementations. This would allow code to try
threadsafe parsing first and only fall back to a main thread bounce if its
an oddball URL. Right now we hardcode a bunch of http/https/nsStandardURL
things in various places to accomplish this.


>
>

[Valentin Gosu]

Yes, that is definitely something we want to fix, but not very
straightforward. We have quite a few URI implementations, and a bunch more
protocol handlers.

https://github.com/valenting/gecko/wiki/Threadsafe-URIs-progress#protocol-handler-implementations