Set memory softlimit in nogvl_context_call also

[masongup-mdsol]

This PR addresses issue #160, where the memory limit was not being enforced when the call method is used to execute Javascript functions. It explicitly sets the MEM_SOFTLIMIT_VALUE and MEM_SOFTLIMIT_REACHED values each time call is used, as is already done in the eval code. I have observed the MEM_SOFTLIMIT_VALUE being overwritten with seemingly random data between Ruby calls, causing the memory limiter to trip either too soon or not at all.

I am not sure if it would be practical to write a Ruby test for this, as we would have to assert that the code terminates with a V8OutOfMemoryError at a specific amount of memory used, and not before or after.

[SamSaffron]

Change looks safe to me... thanks!

Yeah a test is hard here, but yeah maybe if we set the limit low enough and allocate a giant string we can trigger it reliably?

Can you give it a shot, if too hard I can just merge. Key is test needs to be both fast and reliable.

[masongup-mdsol]

I added a test that passes with my change on my system, and fails if I comment out line 1459.

The tricky part isn't that the memory limiter fails to trigger, but that the limit value gets overwritten. I had to add the set_s and context.call('set_s', 1000) code to get it to trigger. On my system, that overwrites the memory limit to zero, causing it to trigger immediately. I can catch that this happened by asserting that the context's s is sufficiently high, so the loop hasn't run a number of times that's consistent with actually hitting the memory limit.

I have also observed the memory limit getting overwritten to a massively high value in the codebase for our project that I originally found this issue in. I am not sure how to trigger that to happen reliably, but I was concerned that it might happen on some other system configuration. If that happens, then the test suite segfaults after that test takes up all of the available memory. To prevent this, I added a bit of Javascript code to exit cleanly if the value gets too high. The test catches this by asserting that the exception was raised, so it fails if this break causes it to exit cleanly.

I think these checks effectively assert that the memory limit is there, and at least roughly in the range that it was explicitly set to. I don't know how the overwrite is happening exactly, but I'm assuming that it's very unlikely that it would be overwritten to a value that would allow this test to pass.

[SamSaffron]

Looks good to me!

[masongup-mdsol]

Excellent, thanks @SamSaffron! Any idea when this can be released to Rubygems in an updated version?

[SamSaffron]

No probs, released a new version just now.

…

On Wed, Apr 22, 2020 at 3:47 AM Mason Gup ***@***.***> wrote: Excellent, thanks @SamSaffron <https://github.com/SamSaffron>! Any idea when this can be released to Rubygems in an updated version? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#161 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAABIXPA4FJHP2VTH4YOG23RNXL2BANCNFSM4MFQ7HFQ> .