Side navigation
#5048 closed bug (patchwelcome)
Opened August 14, 2009 09:30AM UTC
Closed April 01, 2011 06:23PM UTC
Last modified March 13, 2012 06:11PM UTC
Memory leak when jsonp $.ajax() request fails.
| Reported by: | sigmunau | Owned by: | |
|---|---|---|---|
| Priority: | low | Milestone: | 1.next |
| Component: | ajax | Version: | 1.4.3 |
| Keywords: | ajaxrewrite | Cc: | jaubourg |
| Blocked by: | Blocking: |
Description
jsonp ajax requests does some explicit garbage collection when the jsonp request succeeds. but this code is missing from the error paths. Attached patch fixes the memory leak and also fixes a case where error callback isn't run because firefox throws an exception on xhr.open() for certain malformed urls.
Attachments (2)
Change History (10)
Changed August 14, 2009 03:38PM UTC by comment:1
Changed September 28, 2010 09:08AM UTC by comment:2
$.ajax() does not work.
Changed October 21, 2010 01:24AM UTC by comment:3
| blockedby: | → 5201 |
|---|---|
| milestone: | 1.4 → 1.5 |
| priority: | major → high |
| status: | new → open |
| version: | 1.3.2 → 1.4.3 |
Changed December 27, 2010 08:39PM UTC by comment:4
| blockedby: | 5201 |
|---|
This needs to be reverified with the new ajax module.
Changed December 27, 2010 09:09PM UTC by comment:5
| keywords: | → ajaxrewrite |
|---|
Changed January 01, 2011 10:59PM UTC by comment:6
| priority: | high → undecided |
|---|
Changed February 02, 2011 09:42AM UTC by comment:7
$.ajax is seriously flawed after 1.4.2 !!!
protocol = loc.protocol || "http:",
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
protocol may be WRITEONLY, thus this will generate an uncaught exception.
Please fix it !!!!!
Changed February 02, 2011 12:27PM UTC by comment:8
| milestone: | → 1.next |
|---|---|
| priority: | undecided → low |
Replying to [comment:7 anonymous]:
$.ajax is seriously flawed after 1.4.2 !!! protocol = loc.protocol || "http:", ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ protocol may be WRITEONLY, thus this will generate an uncaught exception. Please fix it !!!!!
This comment is totally unrelated to this ticket. Also please use less exclamation marks. You probably are looking for #8138
Changed April 01, 2011 06:08PM UTC by comment:9
| cc: | → jaubourg |
|---|
Unless I'm missing something, we're going to need a new test case that we can use to verify this behaviour with the ajax rewrite that landed in 1.5. CC'ing jaubourg in case he knows whether this is still an issue or not.
Changed April 01, 2011 06:23PM UTC by comment:10
| resolution: | → patchwelcome |
|---|---|
| status: | open → closed |
It will memleak and there is nothing that can be done save from setting a timeout as suggested (which won't work for long-polling).
There is no way to handle errors cross-browser for injected script tags, so any network error will leave the script tag dangling and memory will leak.
We could fix the issue for jsonp but it would involve duplicating code for script tag injection and I'm not sure it is something we want to do.
If you want better cross-domain script loading support, I suggest petitionning browser vendors so that we have a proper mean to load scripts WITHOUT script tag injection (or, at the very least, petition Opera so that they provide the, non-standard-yet-available-everywhere-else, onerror handler).
I'll close this as patch welcome but I passed quite some days to try and figure out a proper solution to no avail.
Second patch uses a trick with a timer to make sure error callback is called, and resources cleaned up when the browser is unable to fetch the jsonp data for some more reasons, including 404 response from server or network issues making the server unreachable. I'm not 100% sure this is correct but it seems to work in my test cases