Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak #25

Open
israellot opened this issue Jan 9, 2017 · 4 comments
Open

Memory leak #25

israellot opened this issue Jan 9, 2017 · 4 comments

Comments

@israellot
Copy link

Theres an issue on this line
responses[response.id] = response.data;
This array never gets cleared.

@israellot
Copy link
Author

There another memory leak on
requests[this.id] = this;
This line return $.extend(new Request(), obj); will trigger the insertion of another property on requests but it will never get removed.

@fleon
Copy link
Contributor

fleon commented Jan 13, 2017

@israellot

Ideally this would have been easy to solve if Javascript had a Dictionary data structure which supported weak keys. That way requests and responses could've been garbage collected automatically if not in use.

It is important to keep the requests and responses persistent to support accessing jQuery and to add suport for chaining with promises in the other frame (undocumented). For instance, this makes the following possible:

please(iframe).$('.some-class').parent().parent().next().html().then(function (html) {
    console.log('html: ', html);
});

I'm not really sure how we could achieve the above without keeping request and response references (or after destroying them).

I guess we could write a manual gc that clears all old requests and responses older than some specific threshold (say 30-60 seconds) every specific interval of time. If some request or response is accessed after that duration, it could throw an error that says "Object expired".

What do you say?

@israellot
Copy link
Author

What I'm saying is that there as ghost references in the Requests due to a constructor that inserts references that won't get used and cleared. That was causing the References object to eventually grow so big it would crash the process as I was making several requests/sec with lots of data. I fixed that and I will make a pull request.
I'm not using jquery requests on my test, just simple calls. In my opinion this library would be much more generic if it didn't rely on jquery. Accessing the dom across frames can be easily solved putting the dom concerning code on the requested frame and returning the result, as a normal request-response. Jquery attachment could be an optional plugin distributed in a separated file. You could rely on native promises or leave the choice up to the user of which promise library to use. I personally have bluebird on my projects and I would like to keep using it and not have jquery deferred. I will take some time and try to put these ideias into a proof of concept on top of your code and pull it as well.
Thank you!

@fleon
Copy link
Contributor

fleon commented Jan 13, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants