Closures + XMLHttpRequest + Memory Leak

Collapse
This topic is closed.
X
X
 
  • Time
  • Show
Clear All
new posts
  • Matt Kruse

    #1

    Closures + XMLHttpRequest + Memory Leak

    I'm aware of the circular reference memory leak problem with IE/closures.
    I'm not sure exactly how to resolve it in this situation. Also, Firefox
    appears to grow its memory size with the same code. So I'm wondering if I'm
    missing something?

    My test code is as follows:

    function myObj() {
    var req = new Object();
    req.temp = 0;
    if (window.XMLHttp Request) { req.xmlHttpRequ est = new XMLHttpRequest( ); }
    else if (window.ActiveX Object) { req.xmlHttpRequ est = new
    ActiveXObject(" Msxml2.XMLHTTP" ); }
    req.xmlHttpRequ est.onreadystat echange =
    function() {
    if (req.readyState ==4) {
    req.temp = req.xmlHttpRequ est.responseTex t;
    }
    };
    req.xmlHttpRequ est.open("GET", "/",true);
    req.xmlHttpRequ est.send(null);
    return req;
    }
    // Create a whole bunch of these objects to check for memory leak
    for (var i=0; i<1000; i++) {
    var x = new myObj();
    }

    What is the best way to avoid memory leaking in this example?

    --
    Matt Kruse



  • Random

    #2
    Re: Closures + XMLHttpRequest + Memory Leak

    Matt Kruse wrote:[color=blue]
    > I'm aware of the circular reference memory leak problem with IE/closures.
    > I'm not sure exactly how to resolve it in this situation. Also, Firefox
    > appears to grow its memory size with the same code. So I'm wondering if I'm
    > missing something?
    >
    > My test code is as follows:
    >
    > function myObj() {
    > var req = new Object();
    > req.temp = 0;
    > if (window.XMLHttp Request) { req.xmlHttpRequ est = new XMLHttpRequest( ); }
    > else if (window.ActiveX Object) { req.xmlHttpRequ est = new
    > ActiveXObject(" Msxml2.XMLHTTP" ); }
    > req.xmlHttpRequ est.onreadystat echange =
    > function() {
    > if (req.readyState ==4) {
    > req.temp = req.xmlHttpRequ est.responseTex t;
    > }
    > };
    > req.xmlHttpRequ est.open("GET", "/",true);
    > req.xmlHttpRequ est.send(null);
    > return req;
    > }
    > // Create a whole bunch of these objects to check for memory leak
    > for (var i=0; i<1000; i++) {
    > var x = new myObj();
    > }
    >
    > What is the best way to avoid memory leaking in this example?
    >
    > --
    > Matt Kruse
    > http://www.JavascriptToolbox.com[/color]


    Not sure if this will solve your problem, but I noticed a few wasted
    resources and unclear code.

    First, when you say:[color=blue]
    > function myObj() {
    > var req = new Object();[/color]

    You are actually instantiating two objects for every call to new myObj.

    Second:[color=blue]
    > req.xmlHttpRequ est.onreadystat echange =
    > function() {
    > if (req.readyState ==4) {
    > req.temp = req.xmlHttpRequ est.responseTex t;
    > }
    > };[/color]

    For everything within the anonymous function, req should be 'this'.

    This is how I would I would rewrite this:
    function makeXMLRequest( URI ) {
    var x = (
    window.XMLHttpR equest ?
    ( new XMLHttpRequest( ) )
    : ( new ActiveXObject( 'Msxml2.XMLHTTP ' ) )
    );

    x.open( 'GET', encodeURI( URI || 'about:blank' ), true );
    x.send( null );

    return x;
    }


    Now a call to x = makeXMLRequest (NOT new makeXMLRequest) will return
    an instance of either XMLHttpRequest OR a new ActiveXObject of type
    'Msxml2.XMLHTTP '.

    This should result in more efficient garbage collection as well.

    The real problem (I think) is that you are creating all of these
    objects and not necessarily giving them time to complete the HTTP
    request. This is probably preventing the necessary garbage collection
    and creating 1000 simultaneous HTTP requests. Ouch.

    Hope that helps.

    Comment

    • Matt Kruse

      #3
      Re: Closures + XMLHttpRequest + Memory Leak

      Random wrote:[color=blue]
      > First, when you say:[color=green]
      >> function myObj() {
      >> var req = new Object();[/color]
      > You are actually instantiating two objects for every call to new
      > myObj.[/color]

      This is a simplified example of my real case, which needs to do this. But
      that's unrelated...
      [color=blue]
      > This is how I would I would rewrite this:[/color]

      Well, that doesn't something entirely different. You can't really solve a
      problem by rewriting it and removing functionality, can you? :)
      [color=blue]
      > Now a call to x = makeXMLRequest (NOT new makeXMLRequest) will return
      > an instance of either XMLHttpRequest OR a new ActiveXObject of type
      > 'Msxml2.XMLHTTP '.
      > This should result in more efficient garbage collection as well.[/color]

      Well that's because you've removed the handling function completely!

      --
      Matt Kruse



      Comment

      • Matt Kruse

        #4
        Re: Closures + XMLHttpRequest + Memory Leak

        Matt Kruse wrote:[color=blue]
        > if (req.readyState ==4) {[/color]

        Oops, this is actually:

        if (req.xmlHttpReq uest.readyState ==4) {

        of course :)

        --
        Matt Kruse



        Comment

        • Richard Cornford

          #5
          Re: Closures + XMLHttpRequest + Memory Leak

          Matt Kruse wrote:[color=blue]
          > I'm aware of the circular reference memory leak problem with
          > IE/closures. I'm not sure exactly how to resolve it in this
          > situation. Also, Firefox appears to grow its memory size with
          > the same code. So I'm wondering if I'm missing something?[/color]
          <snip>[color=blue]
          > function myObj() {
          > var req = new Object();
          > req.temp = 0;
          > if (window.XMLHttp Request) { req.xmlHttpRequ est = new
          > XMLHttpRequest( ); } else if (window.ActiveX Object) {
          > req.xmlHttpRequ est = new ActiveXObject(" Msxml2.XMLHTTP" ); }
          > req.xmlHttpRequ est.onreadystat echange =
          > function() {
          > if (req.readyState ==4) {
          > req.temp = req.xmlHttpRequ est.responseTex t;[/color]

          req.xmlHttpRequ est.onreadystat echange = null;

          - At this point will remove the curricular reference and free the
          closure.
          [color=blue]
          > }
          > };
          > req.xmlHttpRequ est.open("GET", "/",true);
          > req.xmlHttpRequ est.send(null);
          > return req;
          > }[/color]
          <snip>[color=blue]
          > What is the best way to avoid memory leaking in this example?[/color]

          Belt and braces would have you nulling the reference to the
          XMLHttpRequest object once it is finished with.

          Richard.


          Comment

          • Richard Cornford

            #6
            Re: Closures + XMLHttpRequest + Memory Leak

            Richard Cornford wrote:
            <snip>[color=blue]
            > - At this point will remove the curricular reference ...[/color]
            <snip> ^^^^^^^^^^

            That should have been 'circular'.

            Richard.


            Comment

            • Matt Kruse

              #7
              Re: Closures + XMLHttpRequest + Memory Leak

              Richard Cornford wrote:[color=blue]
              > req.xmlHttpRequ est.onreadystat echange = null;
              > - At this point will remove the curricular reference and free the
              > closure.[/color]

              That's what I thought too, but no luck. IE says "type mismatch".

              Instead, I found that this appears to work:

              delete req.xmlHttpRequ est['onreadystatech ange'];

              And in fact, I'm doing this just to be safe:

              delete req.xmlHttpRequ est['onreadystatech ange'];
              req.xmlHttpRequ est = null;
              req = null;
              CollectGarbage( );

              That *appears* to work in IE. I can see the memory usage grow to over 150MB,
              then drop back to 30MB, so I assume the leak is gone. However, I don't know
              of any way to test for sure. Do you?

              --
              Matt Kruse



              Comment

              • Random

                #8
                Re: Closures + XMLHttpRequest + Memory Leak

                Matt Kruse wrote:[color=blue]
                > Random wrote:[color=green]
                > > First, when you say:[color=darkred]
                > >> function myObj() {
                > >> var req = new Object();[/color]
                > > You are actually instantiating two objects for every call to new
                > > myObj.[/color]
                >
                > This is a simplified example of my real case, which needs to do this. But
                > that's unrelated...[/color]

                Clearly I misunderstood. Sorry I didn't catch that based on your
                original post.
                [color=blue]
                >[color=green]
                > > This is how I would I would rewrite this:[/color]
                >
                > Well, that doesn't something entirely different. You can't really solve a
                > problem by rewriting it and removing functionality, can you? :)[/color]

                See above.
                [color=blue][color=green]
                > > Now a call to x = makeXMLRequest (NOT new makeXMLRequest) will return
                > > an instance of either XMLHttpRequest OR a new ActiveXObject of type
                > > 'Msxml2.XMLHTTP '.
                > > This should result in more efficient garbage collection as well.[/color]
                >
                > Well that's because you've removed the handling function completely![/color]

                See above.

                Comment

                • Richard Cornford

                  #9
                  Re: Closures + XMLHttpRequest + Memory Leak

                  Matt Kruse wrote:[color=blue]
                  > Richard Cornford wrote:[color=green]
                  >> req.xmlHttpRequ est.onreadystat echange = null;
                  >> - At this point will remove the curricular reference and
                  >> free the closure.[/color]
                  >
                  > That's what I thought too, but no luck. IE says
                  > "type mismatch".[/color]

                  Yes, you are right. It looks like I went for assigning a reference to a
                  small harmless (and not 'inner') function to clear the closure in my
                  version.

                  <snip>[color=blue]
                  > And in fact, I'm doing this just to be safe:
                  >
                  > delete req.xmlHttpRequ est['onreadystatech ange'];
                  > req.xmlHttpRequ est = null;
                  > req = null;
                  > CollectGarbage( );[/color]
                  <snip>

                  Calling CollectGarbage without verifying its existence will result in
                  errors on browsers that do not support it.

                  Richard.


                  Comment

                  • zilionis@gmail.com

                    #10
                    Re: Closures + XMLHttpRequest + Memory Leak

                    i ussing xmlhttprequest for few years. Typicaly i use sinchronus data
                    trander

                    example of my uses

                    senddata(method ,data,debug,url )
                    debug = shows respondense content

                    myResult = sendData('updat e',{id:1,name:3 4});

                    after this call i get myResult javascript array (or string)

                    for asyncronus can be added callback function

                    Comment

                    • melitonas ant gmail.com

                      #11
                      Re: Closures + XMLHttpRequest + Memory Leak

                      zilionis@gmail. com wrote:[color=blue]
                      > i ussing xmlhttprequest for few years. Typicaly i use sinchronus data
                      > trander
                      >
                      > example of my uses
                      >
                      > senddata(method ,data,debug,url )
                      > debug = shows respondense content
                      >
                      > myResult = sendData('updat e',{id:1,name:3 4});
                      >
                      > after this call i get myResult javascript array (or string)
                      >
                      > for asyncronus can be added callback function[/color]

                      Comment

                      Working...