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

The call method, through nogvl_context_call, does not seem to enforce the max_memory limit #160

Closed
masongup-mdsol opened this issue Apr 8, 2020 · 3 comments

Comments

@masongup-mdsol
Copy link
Contributor

I am still investigating what's happening here, but it appears that the max_memory limit is only enforced when running Javascript code through the exec method - see the code setting the data point on the isolate and the AddGCEpilogueCallback here, in the nogvl_context_eval method. No comparable code seems to be present in the nogvl_context_call method.

The code that I am currently working with loads a few hundred lines of JS via the eval method which defines various Javascript functions, and then calls the defined methods with Ruby objects using the call method. I was hoping to use the max_memory limit in this code to prevent excessive memory consumption, but it does not seem to be effective. I added a log line to the C callback function gc_callback, and the memory limits I set appear to get stomped by the time I run any of the defined functions - when it runs, it thinks that the softlimit is some random huge value.

At this stage, what I am asking is whether there is an intentional reason for this. If this is not an intended behavior, I am open to creating a test exercising this, and writing a fix.

@SamSaffron
Copy link
Collaborator

I don't think this is intentional, would be more than happy to see a fix.

@masongup-mdsol
Copy link
Contributor Author

I've been looking into this some more. Based on what I've seen, it seems that something is overwriting the value for the MEM_SOFTLIMIT_VALUE in the isolate under some conditions. When I tried to write a Ruby test for it in mini_racer, it was overwritten to 0, and would exit on the first GC run. When I was running the original code in the application I'm working, it would get overwritten to a very high value, and would never trigger. I see 2 possible ways to address this issue:

  1. Figure out and fix whatever is overwriting the MEM_SOFTLIMIT_VALUE. I am not an expert in C++, and probably don't have bandwidth to figure that out.

  2. Add a bit of code in nogvl_context_call to set the memory limit if present, just like is already present in nogvl_context_eval. I have implemented this locally and tested it, and it seems to work as expected, both in the test I wrote on mini_racer, and in our application. I'll open a PR with this change.

One other note - it doesn't seem to be possible to write a Ruby test exercising this issue. The memory limiter still runs with or without the fix, it just runs with an incorrect value for MEM_SOFTLIMIT_VALUE, and thus terminates at the wrong time.

@masongup-mdsol
Copy link
Contributor Author

This has been fixed with the linked PR #161, and released in version 0.2.10. Thanks for the support!

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