[Json-rpc-java] Clientside memory issues with the JSON RPC JavaScript

[Arthur Blake]

Hi Eric.
Just a few quick notes.

1. Synchronous calls have been deprecated and we don't recommend using them
at all anymore.  See http://code.google.com/p/jabsorb/issues/detail?id=25

2. We've done quite a bit of refactoring of jsonrpc.js.  If you don't mind
examining the latest version to see if you see these same issues, we'd
appreciate it.  I will do the same when I have time.  If the issues still
exist, we'll create bug tracker entries for them and address them as soon as
possible.
(see http://jabsorb.googlecode.com/svn/trunk/webapps/jsonrpc/jsonrpc.js for
latest version of the code.)

3. I agree that jsonrpc.js is a mess, and I think it needs a total
overhaul... we'll be working on that in upcoming jabsorb releases.

See http://code.google.com/p/jabsorb/ for the latest release of jabsorb,
(formerly known as json-rpc-java.)

On 10/29/07, Eric Pascarello <alienfrog at gmail.com> wrote:
>
> Hello everyone,
>
> I have been working on memory issues with an application and I found some
> major issues within the core javascript of the JSON RPC. There are two
> bugs
> I have found so far and there are other improvements that can take place.
> With this thread I will only talk about the memory issues.
>
> The bugs:
> 1) The object JSONRpcClient.async_inflight is  growing with sychronous
> requests.
> 2) There is a circular reference within all of the Ajax calls.
>
> For the JSONRpcClient.async_inflight, who ever coded it forgot to remove
> the
> current request from the object for synchronous requests. The code
> properly
> removes itt for asychronous requests. You can see that by looking at the
> code within the onreadystatechnage callback.
>
> To test for this bug, fire off a series of Synch requests and with Firebug
> console [http://www.getFirebug.com], type JSONRpcClient.async_inflightinto
> the command line. Doing this will allow you to see what is in the array at
> that given time.
>
> So I changed
>
>     if(!req.cb)
>         return this._handleResponse(http);
>
> to this
>
>     if(!req.cb){
>         delete JSONRpcClient.async_inflight[req.requestId];
>         return this._handleResponse(http);
>     }
>
> and JSONRpcClient.async_inflight is no longer gaining in size. That was an
> easy fix. :)
>
>
> The next issue was a circular reference. If you were to inspect the code,
> and look at the DOM tree, you would see:
>
>
>    - System
>       - listMethods
>          - client
>             - System
>          - listMethods
>             - client
>                      - To Infinity and beyond
>
> Firebug is a great thing because it lets you inspeact the objects. To test
> for this, you can add the line console.log(this); inside of the
> JSONRpcClient constructor I am [thinking it somewhere around 115-118.]
> Click
> on the Object in the console tab and you can navigate the tree and see the
> circular reference first hand.
>
> The circular reference problem is in the following chunk of code in
> _createMethod():
>
> JSONRpcClient.prototype._createMethod =
> function JSONRpcClient_createMethod(methodName)
> {
>     var fn=function()
>     {
>     var args = [];
>     var callback = null;
>     for(var i=0;i< arguments.length;i++) args.push(arguments[i]);
>     if(typeof args[0] == "function") callback = args.shift();
>     var req = fn.client._makeRequest.call(fn.client, fn.methodName,
>                           args, callback);
>     if(callback == null) {
>         return fn.client._sendRequest.call(fn.client, req);
>     } else {
>         JSONRpcClient.async_requests.push(req);
>         JSONRpcClient.kick_async();
>         return req.requestId ;
>     }
>     };
>     fn.client = this;
>     fn.methodName = methodName;
>     return fn;
> };
>
> The whole problem relates to this line fn.client = this;. They than use
> fn.client inside the function to be able to reference the make and send
> methods. Storing this is in an another object is usually a very bad  thing
> to do. What should have been done is used a local variable and reference
> that. This keeps us from causing the endless loop of it refering to itself
> over and over again. Something like this would do the trick:
>
> JSONRpcClient.prototype._createMethod =
> function JSONRpcClient_createMethod(methodName)
> {
>
>     var ref = this;
>
>     var fn=function()
>     {
>     var args = [];
>     var callback = null;
>     for(var i=0;i<arguments.length;i++) args.push(arguments[i]);
>     if(typeof args[0] == "function") callback = args.shift();
>     var req = ref._makeRequest.call(ref, fn.methodName,
>                           args, callback);
>     if(callback == null) {
>         return ref._sendRequest.call(ref, req);
>     } else {
>         JSONRpcClient.async_requests.push(req);
>         JSONRpcClient.kick_async();
>         return req.requestId;
>     }
>     };
>     fn.methodName = methodName;
>     return fn;
> };
>
> Now there is no circular reference when you examine it in the Firebug
> console. I used var ref = this; to allow me to reference this. We no
> longer
> have to reference client in this chunk of code.
>
> Hopefully someone out there will update the project with the fixes so they
> can have some of their memory back. In the future we can talk about
> improving the code in some other areas.
>
> Regards,
> Eric Pascarello
> _______________________________________________
> Json-rpc-java mailing list
> Json-rpc-java at oss.metaparadigm.com
> http://oss.metaparadigm.com/mailman/listinfo/json-rpc-java
>