1009648 - nsStandardURL crashes when calling SetHost after Clear()

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Valentin Gosu [:valentin] (he/him)]

xpcshell-test in test_standardurl.js

function test_clearedSpec()
{
  var url = stringToURL("http://example.com");
  url.spec = "";
  url.host = "example.com";
}

Comment 1

[Valentin Gosu [:valentin] (he/him)]

Attachment: crash_standardurl.patch

This patch changes nsStandardURL so that when SetSpec fails, the old spec is restored and the url object is still valid.

Comment 2

[Valentin Gosu [:valentin] (he/him)]

Attachment: nsStandardURL crashes when calling SetHost after Clear()

This new patch creates a backup nsStandardURL which it uses to revert the URL in case SetSpec fails.
Unlike the previous patch, we don't call SetSpec again, so there's no chance of a stack overflow.
It it however a bit more complex than the old one.

Comment 3

[Patrick McManus [:mcmanus]]

Comment on attachment 8470124 [details] [diff] [review]
nsStandardURL crashes when calling SetHost after Clear()

Review of attachment 8470124 [details] [diff] [review]:
-----------------------------------------------------------------

this is getting more exciting each time..

Comment 4

[Valentin Gosu [:valentin] (he/him)]

https://tbpl.mozilla.org/?tree=Try&rev=72db44f5f499
Try run is green.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc22b1e92554

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Backed out for frequent Windows debug shutdown crashes.
https://hg.mozilla.org/integration/mozilla-inbound/rev/f9825adf25c5

https://tbpl.mozilla.org/php/getParsedLog.php?id=45590548&tree=Mozilla-Inbound

Comment 7

[Valentin Gosu [:valentin] (he/him)]

Attachment: fix_windows_crash.patch

We shouldn't add urls to gAllURLs if the URL is on the stack.
Not sure why this crash only happens on Windows. My guess is that there's a race between SetSpec and ShutdownGlobalObjects.

Comment 8

[Valentin Gosu [:valentin] (he/him)]

No more crashes it seems: https://tbpl.mozilla.org/?tree=Try&rev=27df696692e3

Comment 9

[Valentin Gosu [:valentin] (he/him)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f7ff242d4af8
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffe2d5537d2d

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f7ff242d4af8
https://hg.mozilla.org/mozilla-central/rev/ffe2d5537d2d