Bug Tracker

Modify

Ticket #5048 (closed bug: patchwelcome)

Opened 4 years ago

Last modified 15 months ago

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
Blocking: Blocked by:

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

jquery-1.3.2-jsonp-leak.patch Download (1.6 KB) - added by sigmunau 4 years ago.
jquery-jsonp-404-memleak-fix.patch Download (1.6 KB) - added by sigmunau 4 years ago.
attempted fix for timeout and 404 (etc) errors with jsonp requests

Change History

Changed 4 years ago by sigmunau

Changed 4 years ago by sigmunau

attempted fix for timeout and 404 (etc) errors with jsonp requests

comment:1 Changed 4 years ago by sigmunau

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

comment:2 Changed 3 years ago by somnus

$.ajax() does not work.

comment:3 Changed 3 years ago by snover

  • Priority changed from major to high
  • Status changed from new to open
  • Version changed from 1.3.2 to 1.4.3
  • Blocked by 5201 added
  • Milestone changed from 1.4 to 1.5

comment:4 Changed 2 years ago by rwaldron

  • Blocked by 5201 removed

This needs to be reverified with the new ajax module.

comment:5 Changed 2 years ago by ajpiano

  • Keywords ajaxrewrite added

comment:6 Changed 2 years ago by rwaldron

  • Priority changed from high to undecided

comment:7 follow-up: ↓ 8 Changed 2 years ago by 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 !!!!!

comment:8 in reply to: ↑ 7 Changed 2 years ago by jitter

  • Priority changed from undecided to low
  • Milestone set to 1.next

Replying to 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

comment:9 Changed 2 years ago by addyosmani

  • Cc jaubourg added

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.

comment:10 Changed 2 years ago by jaubourg

  • Status changed from open to closed
  • Resolution set to patchwelcome

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.

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.