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

syscall/js: panic in os.Getpid() when called from wasm in a browser #34627

Closed
Stebalien opened this issue Sep 30, 2019 · 7 comments
Closed

syscall/js: panic in os.Getpid() when called from wasm in a browser #34627

Stebalien opened this issue Sep 30, 2019 · 7 comments
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unplanned

Comments

@Stebalien
Copy link

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

$ go version
go version go1.13 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/steb/.cache/go-build"
GOENV="/home/steb/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/steb/projects/go"
GOPRIVATE=""
GOPROXY="direct"
GOROOT="/usr/lib/go"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/steb/projects/go/src/github.com/golang/go/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/run/user/1000/tmp/go-build550335201=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Tried to use os.Getpid() from WASM. Gist: https://gist.github.com/Stebalien/4c478e71b0264f9dd66dab9676770511

What did you expect to see?

Either the PID or a faked PID when not applicable (e.g., 1).

What did you see instead?

A panic:

--- FAIL: TestInBrowser (0.00s)
panic: syscall/js: call of Value.Get on undefined [recovered]
	panic: syscall/js: call of Value.Get on undefined

goroutine 8 [running]:
testing.tRunner.func1(0x472100)
	/usr/lib/go/src/testing/testing.go:874 +0x38
panic(0x28ca0, 0x40a0a0)
	/usr/lib/go/src/runtime/panic.go:679 +0x20
syscall/js.Value.Get(0x0, 0x4e818, 0x3, 0x5d928fce)
	/usr/lib/go/src/syscall/js/js.go:252 +0x39
syscall.Getpid(0x15c95b24db15e800)
	/usr/lib/go/src/syscall/syscall_js.go:310 +0x2
os.Getpid(...)
	/usr/lib/go/src/os/exec.go:71
foobar.TestInBrowser(0x472100)
	/home/steb/foobar/foobar_test.go:9 +0x4
testing.tRunner(0x472100, 0x57738)
	/usr/lib/go/src/testing/testing.go:909 +0xe
created by testing.(*T).Run
	/usr/lib/go/src/testing/testing.go:960 +0x2d
FAIL	foobar	0.413s
FAIL

The syscall/js package assumes that all wasm instances have "fs" and "process" globals available.
@Stebalien Stebalien changed the title syscall/js: "process" and "fs" globals shouldn't be assumed to exist syscall/js: panic in os.Getpid() when called from wasm in a browser Sep 30, 2019
Stebalien added a commit to whyrusleeping/go-logging that referenced this issue Sep 30, 2019
@Stebalien
fix: work around golang/go#34627
a7837d3 
os.Getpid() will panic on wasm in browsers (but not in node).

Issue: golang/go#34627
@Stebalien Stebalien mentioned this issue Sep 30, 2019
Merged
Stebalien added a commit to whyrusleeping/go-logging that referenced this issue Sep 30, 2019
@Stebalien
fix: work around golang/go#34627
3c45b0f 
os.Getpid() will panic on wasm in browsers (but not in node).

Issue: golang/go#34627
Stebalien added a commit to whyrusleeping/go-logging that referenced this issue Sep 30, 2019
@Stebalien
Merge pull request #4 from whyrusleeping/fix/wasm
d89ec39 
fix: work around golang/go#34627
@agnivade
Copy link
Contributor

agnivade commented Oct 1, 2019

The syscall/js package assumes that all wasm instances have "fs" and "process" globals available.

Correct. The syscall package is not aware of whether it is being called inside a browser or node. Hence it crashes when you try to access the process instance. That is the behavior for all wasm related operations, including the ones in the os package.

@neelance - Do you think it makes sense to add a package level doc mentioning that any function which does not have the corresponding functionality in the target environment will panic ? I think intentional panics should be documented somewhere.

@Stebalien
Copy link
Author

If these panics are intentional, they should be an explicit panic("not supported on the current platform") (or something like that).

However, I strongly disagree that they should panic. os.Getuid() doesn't make sense on Windows but it doesn't panic, it just returns -1. I'd expect functions that don't make sense on a platform to return sane values or errors.

@andybons andybons added arch-wasm WebAssembly issues NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 1, 2019
@andybons andybons added this to the Unplanned milestone Oct 1, 2019
@janpfeifer
Copy link

+1 to return some form of error ...

I just found out this broke github.com/golang/glog in wasm :\

@gopherbot
Copy link

Change https://golang.org/cl/199357 mentions this issue: syscall: on wasm, do not panic if "process" global is not defined

@gopherbot
Copy link

Change https://golang.org/cl/199698 mentions this issue: syscall: on wasm, do not panic if "process" global is not defined

@janpfeifer
Copy link

QQ: did the fix made into 1.13.3 ? (sorry, the release notes don't mention it, but I'm not sure how the process works)

@agnivade
Copy link
Contributor

It did not. Patch releases are only for security fixes and serious regressions. You will have to wait for 1.14, or alternatively use tip.

@golang golang locked and limited conversation to collaborators Oct 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-wasm WebAssembly issues FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
5 participants
@Stebalien @andybons @agnivade @janpfeifer @gopherbot