Threadsafe URIs in Gecko

Valentin Goșu

valentin@mozilla.com

nsIURI is not thread safe

Hack around it:

Bounce to the main thread to access the URI

because addons

var url = {
    counter: 0,
    password: "",
    asciiSpec:
        function() { counter++; return "spec" + counter; },
    QueryInterface: XPCOMUtils.generateQI([Ci.nsIURI])
}

We now control all 11 nsIURI implementations

nsStandardURL
  -> SubstitutingURL
nsSimpleURI
  -> nsSimpleNestedURI
      -> nsNestedAboutURI
  -> nsHostObjectURI
  -> nsJSURI
nsJARURI
NullPrincipalURI
nsMozIconURI
  -> nsNestedMozIconURI

6864 lines containing "nsIURI"

Hurray for Quantum!

// Thread 1
url.spec = "http://test.com/path"
if (url.scheme == "http") {
    url.hostname = "other.com"
}
// We expect url.spec == "http://other.com/path"

// Thread 2

url.spec = "ftp://kernel.org/file"

// We expect url.spec == "ftp://kernel.org/file"

Actual results may be any of:
"http://other.com/path"
"ftp://kernel.org/file"
"ftp://other.com/file"

interface nsIURI : nsISupports
{
    [...]
    nsIURIMutator mutate();
}

interface nsIURIMutator : nsISupports
{
    nsIURIMutator setScheme(in AUTF8String aScheme);
    nsIURIMutator setUserPass(in AUTF8String aUserPass);
    [...]
    nsIURI finalize();
}

Step 1: Add new nsIURIMutator API [done]*

Step 2: Update all the places where we call nsIURI setters

Step 2b: Add warnings/assertions for all nsIURI setters

Step 3: Make nsIURI attributes readonly

Follow up:

Step 4: Centralize all URI parsers

Step 5: Replace all with rust-url

function DO_NOT_DO_THIS(uri) {
    uri.query = "hello";
    uri.ref = "bla";
    uri.scheme = "ftp";
    return uri; // you just changed the initial URI!
}

function oldWay(uri) {
    let newURI = uri.clone();
    newURI.query = "hello";
    newURI.ref = "bla";
    newURI.scheme = "ftp";
    return newURI;
}

function betterWay(uri) {
    return uri.mutate()
              .setQuery("hello")
              .setRef("bla")
              .setScheme("ftp")
              .finalize();
}

nsresult DO_NOT_DO_THIS(nsIURI* aURI, nsIURI** changedURI) {
    nsCOMPtr<nsIURI> newURI = aURI;
    nsresult rv = newURI->SetRef(NS_LITERAL_CSTRING("test"));
    newURI.forget(changedURI); // we changed the initial URI
    return rv;
}

nsresult OldWay(nsIURI* aURI, nsIURI** changedURI) {
    nsCOMPtr<nsIURI> newURI;
    nsresult rv = aURI->Clone(getter_AddRefs(newURI));
    NS_ENSURE_SUCCESS(rv, rv);
    rv = newURI->SetRef(NS_LITERAL_CSTRING("test"));
    newURI.forget(changedURI);
    return rv;
}

#include "nsIURIMutator.h"
nsresult BetterWay(nsIURI* aURI, nsIURI** changedURI) {
    nsCOMPtr<nsIURI> newURI;
    nsresult rv = NS_MutateURI(uri)
                    .SetRef(NS_LITERAL_CSTRING("test"));
                    .Finalize(newURI);
    newURI.forget(changedURI);
    return rv;
}

#include "nsIURIMutator.h"
nsresult BetterWay(nsIURI* aURI, nsIURI** changedURI) {
    nsCOMPtr<nsIURI> newURI;
    nsresult rv = NS_MutateURI(uri)
                    .SetRef(NS_LITERAL_CSTRING("test"));
                    .Finalize(newURI);
    newURI.forget(changedURI);
    return rv;
}

#include "nsIURIMutator.h"
nsresult EvenBetterWay(nsIURI* aURI, nsIURI** changedURI) {
    // after patch lands
    return NS_MutateURI(uri)
             .SetRef(NS_LITERAL_CSTRING("test"));
             .Finalize(changedURI);
}

Help us make nsIURI threadsafe by:

• Write new code using the nsIURIMutator API
• Change old code to use nsIURIMutator

Helpful resources:

nsIURIMutator.idl

TestStandardURL.cpp::Mutator

test_uri_mutator.js