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

syscall: js/wasm file operations fail on windows / node.js #71758

Closed
Zxilly opened this issue Feb 14, 2025 · 9 comments
Closed

syscall: js/wasm file operations fail on windows / node.js #71758

Zxilly opened this issue Feb 14, 2025 · 9 comments
Labels
arch-wasm WebAssembly issues compiler/runtime Issues related to the Go compiler and/or runtime. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. OS-JS Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@Zxilly
Copy link
Member

Zxilly commented Feb 14, 2025

Go version

go version go1.24.0 windows/amd64

Output of go env in your module/workspace:

set AR=ar
set CC=gcc
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_ENABLED=1
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set CXX=g++
set GCCGO=gccgo
set GO111MODULE=
set GOAMD64=v1
set GOARCH=amd64
set GOAUTH=netrc
set GOBIN=
set GOCACHE=C:\Users\zxilly\AppData\Local\go-build
set GOCACHEPROG=
set GODEBUG=
set GOENV=C:\Users\zxilly\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFIPS140=off
set GOFLAGS=
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users\zxilly\AppData\Local\Temp\go-build2393991947=/tmp/go-build -gno-record-gcc-switches
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMOD=T:\go-size-analyzer\go.mod
set GOMODCACHE=C:\Users\zxilly\go\pkg\mod
set GONOPROXY=1
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\zxilly\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Program Files\Go
set GOSUMDB=sum.golang.org
set GOTELEMETRY=local
set GOTELEMETRYDIR=C:\Users\zxilly\AppData\Roaming\go\telemetry
set GOTMPDIR=
set GOTOOLCHAIN=auto
set GOTOOLDIR=C:\Program Files\Go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=go1.24.0
set GOWORK=
set PKG_CONFIG=pkg-config

What did you do?

Run a wasm binary contains file operations on Windows Node.js

What did you see happen?

panic: syscall/js: call of Value.Int on undefined

goroutine 1 [running]:
syscall/js.Value.float({{}, 0x0, 0x0}, {0x6dff7, 0x9})
	C:/hostedtoolcache/windows/go/1.24.0/x64/src/syscall/js/js.go:523 +0x10
syscall/js.Value.Int(...)
	C:/hostedtoolcache/windows/go/1.24.0/x64/src/syscall/js/js.go:540
syscall.init()
	C:/hostedtoolcache/windows/go/1.24.0/x64/src/syscall/fs_js.go:32 +0x37
C:\Program Files\nodejs\node.exe [--stack-size=8192 C:\hostedtoolcache\windows\go\1.24.0\x64\lib\wasm\wasm_exec_node.js C:\Users\RUNNER~1\AppData\Local\Temp\go-build2721495951\b002\go-size-analyzer.test -test.paniconexit0 -test.gocoverdir=C:\Users\RUNNER~1\AppData\Local\Temp\go-build2721495951\b002\gocoverdir -test.timeout=10m0s -test.v=true -test.gocoverdir=D:\a\go-size-analyzer\go-size-analyzer\covdata\unit]
panic: exit status 2

goroutine 1 [running]:
main.main()
	C:/Users/runneradmin/go/pkg/mod/github.com/!zxilly/go_js_wasm_exec@v1.1.3-0.20250214222407-149a70ce8586/main.go:52 +0x388
exit status 2
FAIL	github.com/Zxilly/go-size-analyzer	8.873s

What did you expect to see?

Works correctly.

@Zxilly
Copy link
Member Author

Zxilly commented Feb 14, 2025

cc @golang/js

@Zxilly
Copy link
Member Author

Zxilly commented Feb 14, 2025

The main problem is that O_DIRECTORY is not available on all platforms, as described at https://nodejs.org/docs/latest/api/fs.html#file-open-constants .

On Windows, only O_APPEND, O_CREAT, O_EXCL, O_RDONLY, O_RDWR, O_TRUNC, O_WRONLY, and UV_FS_O_FILEMAP are available.

However, src/syscall/fs_js.go#32 defines nodeDIRECTORY as constants.Get("O_DIRECTORY").Int(), this leads to an un-null pointer on initialization, which crashes.

I think a possible fix might be to set its initial value to -1 and manually assign it a value in an init?

@seankhliao seankhliao changed the title js/wasm: The js/wasm binary containing file operations always crashes when run on Windows Node.js syscall: js/wasm file operations fail on windows / node.js Feb 14, 2025
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 14, 2025
@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. arch-wasm WebAssembly issues OS-JS labels Feb 14, 2025
@johanbrandhorst
Copy link
Member

Could you try it and submit a CL? Thanks.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/650015 mentions this issue: syscall: disable O_DIRECTORY on Windows for js/wasm

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Feb 16, 2025
@Zxilly
Copy link
Member Author

Zxilly commented Feb 18, 2025

Can we backport this to 1.24?

@dmitshur
Copy link
Contributor

dmitshur commented Feb 26, 2025

@gopherbot Please add this issue for us to consider for backport to 1.24.

I can be convinced otherwise, but not sure if this meets the bar: the experimental js/wasm port is intended to run in browsers, and uses Node only for testing. This breaks testing support on a Windows host. On the other hand, the backport is very safe and there's no workaround for the affected environment other than modifying the files manually.

@gopherbot
Copy link
Contributor

Backport issue(s) opened: #71977 (for 1.24).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@prattmic prattmic added the Testing An issue that has been verified to require only test changes, not just a test failure. label Feb 26, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/652835 mentions this issue: [release-branch.go1.24] syscall: disable O_DIRECTORY on Windows for js/wasm

gopherbot pushed a commit that referenced this issue Feb 26, 2025
…s/wasm

O_DIRECTORY is not available on all platforms, as described at

https://nodejs.org/docs/latest/api/fs.html#file-open-constants .

On Windows, only O_APPEND, O_CREAT, O_EXCL, O_RDONLY, O_RDWR,
O_TRUNC, O_WRONLY, and UV_FS_O_FILEMAP are available.

For #71758.
Fixes #71977.

Change-Id: Iacc890ba9a30dcd75eb746ec324fa0c3e368048e
GitHub-Last-Rev: a0160e8
GitHub-Pull-Request: #71770
Reviewed-on: https://go-review.googlesource.com/c/go/+/650015
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
(cherry picked from commit ad8b330)
Reviewed-on: https://go-review.googlesource.com/c/go/+/652835
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/658295 mentions this issue: syscall: ignore O_DIRECTORY on Windows for js/wasm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly issues compiler/runtime Issues related to the Go compiler and/or runtime. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. OS-JS Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants