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.

Sign up for GitHub

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

Jump to bottom

x/exp/jsonrpc2: panic when sending request and JSON marshaling fails #61654

Closed
maxatome opened this issue Jul 30, 2023 · 5 comments
Closed

x/exp/jsonrpc2: panic when sending request and JSON marshaling fails #61654

maxatome opened this issue Jul 30, 2023 · 5 comments
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Unreleased

Comments

@maxatome
Copy link

What version of Go are you using (go version)?

playground 1.20 and "dev branch"

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

playground

What did you do?

https://go.dev/play/p/4jpZ0wBGCTd

What did you expect to see?

Not a panic.

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
	panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x526783]

goroutine 6 [running]:
testing.tRunner.func1.2({0x5500a0, 0x68fc50})
	/usr/local/go-faketime/src/testing/testing.go:1545 +0x238
testing.tRunner.func1()
	/usr/local/go-faketime/src/testing/testing.go:1548 +0x397
panic({0x5500a0?, 0x68fc50?})
	/usr/local/go-faketime/src/runtime/panic.go:914 +0x21f
golang.org/x/exp/event.New({0x0, 0x0}, 0x4)
	/tmp/gopath3654295953/pkg/mod/golang.org/x/exp/event@v0.0.0-20220217172124-1812c5b45e43/event.go:64 +0x43
golang.org/x/exp/event.End({0x0, 0x0}, {0xc000093d18, 0x1, 0xc000070d40?})
	/tmp/gopath3654295953/pkg/mod/golang.org/x/exp/event@v0.0.0-20220217172124-1812c5b45e43/common.go:126 +0x39
golang.org/x/exp/jsonrpc2.(*AsyncCall).Await(0xc00009e090, {0x5af630, 0x6c7660}, {0x0, 0x0})
	/tmp/gopath3654295953/pkg/mod/golang.org/x/exp/jsonrpc2@v0.0.0-20230728194245-b0cb94b80691/conn.go:219 +0x3ce
play.TestJRPCBug(0xc000007860)
	/tmp/sandbox3486629624/prog_test.go:36 +0x1dc
testing.tRunner(0xc000007860, 0x584538)
	/usr/local/go-faketime/src/testing/testing.go:1595 +0xff
created by testing.(*T).Run in goroutine 1
	/usr/local/go-faketime/src/testing/testing.go:1648 +0x3ad
@gopherbot gopherbot added this to the Unreleased milestone Jul 30, 2023
@maxatome
Copy link
Author

2 fixes are possible:

  1. initialize ctx field at https://cs.opensource.google/go/x/exp/+/master:jsonrpc2/conn.go;l=146
		resultBox: make(chan asyncResult, 1),
+		ctx:       ctx,
 	}
  1. move the https://cs.opensource.google/go/x/exp/+/master:jsonrpc2/conn.go;l=155-158 block before https://cs.opensource.google/go/x/exp/+/master:jsonrpc2/conn.go;l=148

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 4, 2023
@dr2chase
Copy link
Contributor

dr2chase commented Aug 4, 2023

@jba @ianthehat

@bcmills
Copy link
Contributor

bcmills commented Aug 8, 2023

Option (2) seems reasonable to me. Want to send a CL?

@bcmills bcmills added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Aug 8, 2023
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 8, 2023
@gopherbot
Copy link

Change https://go.dev/cl/516556 mentions this issue: jsonrpc2: do not panic when sending unmarshalable params

@maxatome
Copy link
Author

maxatome commented Aug 8, 2023

Sure: https://go-review.googlesource.com/c/exp/+/516556

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@maxatome @dr2chase @bcmills @gopherbot