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

eth/tracers: do the JSON serialization via .js to capture C faults #22857

Merged
merged 1 commit into from May 11, 2021

Conversation

karalabe
Copy link
Member

@karalabe karalabe commented May 11, 2021

Supersedes #22725.

Credits for the repro test case and the fix idea go to @holiman, I just found it easier to implement the thing in a new PR than to keep hacking one that already did a non-ideal fix.

Duktape's JSON serializer has some hard coded depth limits, currently set at 1000. Any object deeper than that fails to serialize, resulting in a C++ exception. Unfortunately Go can't catch that exception, so the tracer (along with Geth) panics.

Duktape itself has a duk_safe_call method, the purpose of which is exactly to allow catching these exceptions in client code that would otherwise be unable to do, however the wrapper in go-duktape did a dumb C wrapping which makes this method unusable from Go (i.e. fn can't be meaningfully filled since we don't have access to the duk_xxx methods):

func (d *Context) SafeCall(fn, args *[0]byte, nargs, nrets int) int {
	return int(C.duk_safe_call(
		d.duk_context,
		fn,
		unsafe.Pointer(&args),
		C.duk_idx_t(nargs),
		C.duk_idx_t(nrets),
	))
}

Our last option is to use the JavaScript JSON.stringify method to do the flattening. This still uses the C code underneath, so it is equally as fast, but it also does the C++ exception handling, so it's also safe. Duktape is also weird, so this PR also gets weird.

Duktape can only call functions and methods on objects that are already in its heap/stack. Getting the JSON.stringify method on there turned out to be strangely funky. Currently the best idea I had was to push a (JSON.stringify) evaluation onto the .js stack, which when evaluated will push the actual method we want to call onto the stack (and not the string of it). Then I can call this method to do the real work. This whole dance needs to be special cased to be only done when calling result, since whereas JsonEncode was always fast, this extra .js object conversion is very slow, so we can't afford to do it once per step.

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

geth process is stopped because debug_traceBlockByNumber geth crashes while tracing a specific transaction
2 participants