api: introduce SealHandleScope

api: introduce SealHandleScope

When debugging Handle leaks in io.js we found it very convenient to be
able to Seal some specific (root in our case) scope to prevent Handle
allocations in it, and easily find leakage.

I wonder if this kind of API could be exposed?

R=yangguo
BUG=

[indutny, 2015-03-06 19:35:43]

Yang,

This is what we was talking about. Please let me know if the change itself looks
fine and I'll submit the test. Otherwise it appears to be working fine in io.js

Cheers!

[indutny, 2015-03-06 19:36:34]

fedor.indutny@gmail.com changed reviewers:
+ danno@chromium.org, yangguo@chromium.org
- yangguo@google.com

[indutny, 2015-03-06 19:36:47]

Gosh, messed up reviewers, sorry!

[Yang, 2015-03-10 12:52:33]

LGTM, but I would strongly advise for a test case that is marked as always
failing in cctest.status.

https://codereview.chromium.org/985873002/diff/1/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/985873002/diff/1/src/api.cc#newcode700
src/api.cc:700: "HandleScope::HandleScope",
Please update this line to SealHandleScope.

[indutny, 2015-03-11 18:43:03]

Sure, will do it in a bit.

[indutny, 2015-03-11 18:43:14]

And thanks a lot! :)

[indutny, 2015-03-11 22:04:53]

Yang,

Updated the CL. Please note that I have changed DCHECK in handles-inl.h to
CHECK, to make sure that it fails in both debug and release modes.

Thank you!

[Yang, 2015-03-12 04:48:43]

On 2015/03/11 22:04:53, indutny wrote:
> Yang,
> 
> Updated the CL. Please note that I have changed DCHECK in handles-inl.h to
> CHECK, to make sure that it fails in both debug and release modes.
> 
> Thank you!

Please do not do that. That would directly affect all embedders of V8 regarding
performance. Creating handles can be performance critical in a few places.

Why don't you set the limit of the current handle block, like in the internal
implementation of SealHandleScope? The idea there is that when we attempt to
Extend, we run into the level check there. Since we have to do the limit check
either way, this won't make creating a handle more expensive. You can remove the
CHECK in CreateHandle.

[indutny, 2015-03-12 21:07:17]

Sure, PTAL.

Sorry, it took a bit of effort to figure out why it was crashing, until I have
found those lines in DeleteExtensions(). Let me know if it could be improved.

Thank you!

[Yang, 2015-03-13 07:18:20]

LGTM with comments.

https://codereview.chromium.org/985873002/diff/40001/src/api.cc
File src/api.cc (right):

https://codereview.chromium.org/985873002/diff/40001/src/api.cc#newcode701
src/api.cc:701: "Entering the V8 API without proper locking in place");
Do we actually need this check here? If you want to create a handle, what you
need is a HandleScope, which would check for locking. By having a
SealHandleScope, we would explicitly require a new HandleScope if someone wants
to create a handle.

https://codereview.chromium.org/985873002/diff/40001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

https://codereview.chromium.org/985873002/diff/40001/test/cctest/test-api.cc#...
test/cctest/test-api.cc:21661: CHECK(!obj.IsEmpty());
You can use USE(obj);

https://codereview.chromium.org/985873002/diff/40001/test/cctest/test-api.cc#...
test/cctest/test-api.cc:21679: CHECK(!obj.IsEmpty());
Same here.

[indutny, 2015-03-16 04:08:13]

Yang, sorry for delay. Updated.

[Yang, 2015-03-16 07:25:43]

On 2015/03/16 04:08:13, indutny wrote:
> Yang, sorry for delay. Updated.

LGTM.

[Yang, 2015-04-10 20:47:11]

On 2015/03/16 07:25:43, Yang wrote:
> On 2015/03/16 04:08:13, indutny wrote:
> > Yang, sorry for delay. Updated.
> 
> LGTM.

Fedor, do you want to land this? You can simply check the "Commit" checkbox.

[indutny, 2015-04-10 21:14:37]

The CQ bit was checked by fedor.indutny@gmail.com

[indutny, 2015-04-10 21:14:51]

Haha, really? Didn't know about it!

Thank you!

[commit-bot: I haz the power, 2015-04-10 21:15:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/985873002/60001

[commit-bot: I haz the power, 2015-04-10 21:19:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-04-10 21:19:40]

Try jobs failed on following builders:
  v8_presubmit on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_presubmit/builds/1909)

[indutny, 2015-04-10 21:49:59]

Yang, was it wrong email?

[indutny, 2015-04-10 21:58:09]

Opened https://codereview.chromium.org/1079713002/, just in case.

[Yang, 2015-04-10 21:59:43]

On 2015/04/10 21:49:59, indutny wrote:
> Yang, was it wrong email?

Yeah. It seems like you haven't signed the CLA with this email. But you did with
fedor@indutny.com.