CosmWasm / wasmvm

Go bindings to the running cosmwasm contracts with wasmer
Apache License 2.0
169 stars 100 forks source link

Add directory locking mechanism #500

Closed webmaster128 closed 6 months ago

webmaster128 commented 6 months ago

In the past we have seen cases in which multiple instaces ran on the same directory in parallel (especially test setups), leading to unreliable and hard to debug issues down the road.

webmaster128 commented 6 months ago

Bit unfortunate that this is Unix-only, but I guess we can always add Windows support later if we continue working on that.

Oh, interesting. Do you have more information we can add ass a comment or something? I was assuming this is a wrapper around some system specific locking. The Rust one has some sort of Windows support too.

chipshort commented 6 months ago

I don't know about file locking specifically, but the Rust stdlib tries to provide cross-platform wrappers wherever possible and all platform specific stuff is clearly marked in corresponding packages. This Go package however is completely platform-specific. It is not mentioned very clearly in the syscall module docs, but if you change the platform in the dropdown on the right to windows, you get completely different functions. Also, now that I look at it more closely, the docs state:

Deprecated: this package is locked down. Callers should use the corresponding package in the golang.org/x/sys repository instead. That is also where updates required by new systems or versions should be applied. See https://golang.org/s/go1.4-syscall for more information.

So, maybe we should use sys/unix instead? If we want cross-platform support, there also seem to be some packages that provide such a wrapper, e.g. https://pkg.go.dev/github.com/gofrs/flock

webmaster128 commented 6 months ago

Oh that's great insights. I was not aware that those different APIs per target platform exist. Reading through https://go.googlesource.com/proposal/+/refs/heads/master/design/freeze-syscall.md makes me think using golang.org/x/sys/unix is a good approach as it makes explicit what we do here. We can build a cross-platform solution ourselves if needed one day. But right now all of internal/api cannot work on Windows since it requires a working libwasmvm build which we don't have.