Reviewers: Yang,
https://codereview.chromium.org/1310253002/diff/1/src/json-parser.h
File src/json-parser.h (right):
https://codereview.chromium.org/1310253002/diff/1/src/json-parser.h#newcode267
src/json-parser.h:267: ExecutionAccess access(isolate_);
Aehm, are we sure that acquiring the lock here is correct? This will
hold on to the lock while the GC is active. This looks majorly scary to
me. The lock should only be needed to do the interrupt checking, which
is done inside StackGuard::CheckAndClearInterrupt where the lock is
acquired explicitly.
Description:
Move StackGuard::InterruptRequested into StackLimitCheck.
[email protected]
Please review this at https://codereview.chromium.org/1310253002/
Base URL: https://chromium.googlesource.com/v8/v8.git@local_issue-cr-522380
Affected files (+11, -8 lines):
M src/execution.h
M src/execution.cc
M src/isolate.h
M src/json-parser.h
Index: src/execution.cc
diff --git a/src/execution.cc b/src/execution.cc
index
cb14a36c8fbe4d974c1b9fa0ae69a21f5bfa5f8b..4ab2403e56f0a0cc9d9345e51e0a233a260ff21c
100644
--- a/src/execution.cc
+++ b/src/execution.cc
@@ -639,7 +639,7 @@ Handle<String>
Execution::GetStackTraceLine(Handle<Object> recv,
}
-void StackGuard::CheckAndHandleGCInterrupt() {
+void StackGuard::HandleGCInterrupt() {
if (CheckAndClearInterrupt(GC_REQUEST)) {
isolate_->heap()->HandleGCRequest();
}
Index: src/execution.h
diff --git a/src/execution.h b/src/execution.h
index
d783e5c28b80b7fa34dbf989418665ecd5fe9550..2c07a64aaecc5409064ba1bc5eaf2f2edcf73880
100644
--- a/src/execution.h
+++ b/src/execution.h
@@ -200,10 +200,7 @@ class StackGuard final {
// If the stack guard is triggered, but it is not an actual
// stack overflow, then handle the interruption accordingly.
Object* HandleInterrupts();
-
- bool InterruptRequested() { return GetCurrentStackPosition() < climit();
}
-
- void CheckAndHandleGCInterrupt();
+ void HandleGCInterrupt();
private:
StackGuard();
Index: src/isolate.h
diff --git a/src/isolate.h b/src/isolate.h
index
5339bcce5a0b4f656f6a85bd19f56f442c1c406b..e2187be3f9f05ee48de99971ee8aaba47366d466
100644
--- a/src/isolate.h
+++ b/src/isolate.h
@@ -1478,11 +1478,17 @@ class StackLimitCheck BASE_EMBEDDED {
explicit StackLimitCheck(Isolate* isolate) : isolate_(isolate) { }
// Use this to check for stack-overflows in C++ code.
- inline bool HasOverflowed() const {
+ bool HasOverflowed() const {
StackGuard* stack_guard = isolate_->stack_guard();
return GetCurrentStackPosition() < stack_guard->real_climit();
}
+ // Use this to check for interrupt request in C++ code.
+ bool InterruptRequested() {
+ StackGuard* stack_guard = isolate_->stack_guard();
+ return GetCurrentStackPosition() < stack_guard->climit();
+ }
+
// Use this to check for stack-overflow when entering runtime from JS
code.
bool JsHasOverflowed(uintptr_t gap = 0) const;
Index: src/json-parser.h
diff --git a/src/json-parser.h b/src/json-parser.h
index
81c83bd1d8aad08f2d49489b79fc7111f5de4879..3ee874930411579ed88fd067237e3cd491f2ddcd
100644
--- a/src/json-parser.h
+++ b/src/json-parser.h
@@ -263,10 +263,10 @@ Handle<Object>
JsonParser<seq_one_byte>::ParseJsonValue() {
return Handle<Object>::null();
}
- if (isolate_->stack_guard()->InterruptRequested()) {
+ if (stack_check.InterruptRequested()) {
ExecutionAccess access(isolate_);
// Avoid blocking GC in long running parser (v8:3974).
- isolate_->stack_guard()->CheckAndHandleGCInterrupt();
+ isolate_->stack_guard()->HandleGCInterrupt();
}
if (c0_ == '"') return ParseJsonString();
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
