Split handle scopes into an internal version and a version accessible...

Split handle scopes into an internal version and a version accessible
through the API. This allows us to verify state on entry through the API.
In this change verification in the API entry is checking that the current
thread holds the V8 lock when a HandleScope is instantiated if a v8::Locker
has ever been used by the V8 instance.

Committed: http://code.google.com/p/v8/source/detail?r=1140

[iposva, 2009-01-23 07:26:25]

Minor changes to the API and some code movement to enable an internal
HandleScope. Please verify that the API addition makes sense.

And guess what? The verification already turned up at least one missing
v8::Locker in d8.cc. I am looking at fixing the -p support in d8.cc, I will roll
the fix for the missing locker in with that.

-Ivan

[Mads Ager (chromium), 2009-01-23 07:59:29]

LGTM.  With this structure, it is easy make the locker check happen only in
debug mode if it turns out to be expensive in release mode.

http://codereview.chromium.org/18707/diff/215/21
File src/api.cc (right):

http://codereview.chromium.org/18707/diff/215/21#newcode77
Line 77: #define API_ENTRY_CHECK(msg) \
Right align the '\'s for consistency?

[Christian Plesner Hansen, 2009-01-23 10:26:13]

Under the assumption that we need assertions in release mode, LGTM.

However, I'm not convinced that it's sound to have assertions in the version we
distribute, regardless of how small a performance cost there is.  At least, you
need to be sure that the process is always going to crash very soon after anyway
for this to be okay.

http://codereview.chromium.org/18707/diff/33/232
File src/apiutils.h (right):

http://codereview.chromium.org/18707/diff/33/232#newcode31
Line 31: namespace v8 {
Shouldn't this code be in v8::internal?  Namespace v8 is for stuff that is
visible through the api.

Also, why is this in a separate file?  Circular dependencies?

[iposva, 2009-01-23 16:39:46]

Please realize that the verification code is not using CHECK but it is using
ApiCheck which is already used in many places in the API. This is to avoid
crashing (which as you point out is not-desirable) but it notifies the embedding
program to deal with the error in the appropriate fashion.

-Ivan

http://codereview.chromium.org/18707/diff/215/21
File src/api.cc (right):

http://codereview.chromium.org/18707/diff/215/21#newcode77
Line 77: #define API_ENTRY_CHECK(msg) \
On 2009/01/23 07:59:29, Mads Ager wrote:
> Right align the '\'s for consistency?

Done.

http://codereview.chromium.org/18707/diff/33/232
File src/apiutils.h (right):

http://codereview.chromium.org/18707/diff/33/232#newcode31
Line 31: namespace v8 {
On 2009/01/23 10:26:13, Christian Plesner Hansen wrote:
> Shouldn't this code be in v8::internal?  Namespace v8 is for stuff that is
> visible through the api.
> 
> Also, why is this in a separate file?  Circular dependencies?

ImplementationUtilities just moved to a separate file. Which is as you suggest
due to a circular dependency.

ImplementationUtilities is in the v8 namespace because it bridges between the
v8::internal:: and v8:: namespaces. The external API refers to
ImplementationUtilities to allow friend access from v8::internal to otherwise
private declarations.