Closed Bug 1009648 Opened 11 years ago Closed 11 years ago

nsStandardURL crashes when calling SetHost after Clear()

Categories

(Core :: Networking, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: valentin, Assigned: valentin)

Details

Attachments

(2 files, 1 obsolete file)

xpcshell-test in test_standardurl.js function test_clearedSpec() { var url = stringToURL("http://example.com"); url.spec = ""; url.host = "example.com"; }
Attached patch crash_standardurl.patch (obsolete) — Splinter Review
This patch changes nsStandardURL so that when SetSpec fails, the old spec is restored and the url object is still valid.
Attachment #8424260 - Flags: review?(mcmanus)
Attachment #8424260 - Flags: review?(mcmanus) → review+
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.
Attachment #8470124 - Flags: review?(mcmanus)
Attachment #8424260 - Attachment is obsolete: true
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..
Attachment #8470124 - Flags: review?(mcmanus) → review+
Keywords: checkin-needed
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.
Attachment #8470436 - Flags: review?(mcmanus)
Attachment #8470436 - Flags: review?(mcmanus) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: