fix: Do not close wazero module on error (#3758)

[edman]

Fixes #3758

wazero takes care not to leak modules when InstantiateModule returns an error.

This means that sqlc will call result.Close a second time if a non nil module was returned, which causes a segfault.

Edit: this statement is wrong. See my comment below for the correct explanation of the segfault.

[kyleconroy]

wazero takes care not to leak modules when InstantiateModule returns an error.

So does wazero return a valid module and an error message in these cases? checkError also looks at stderr, which make be written to if InstantiateModule does not return an error. Merged as is we wouldn't call result.Close in all cases.

[edman]

So does wazero return a valid module and an error message in these cases?

Ok so.. Your comment made me go back and track more closely what happened. And I found sqlc actually tripped this https://go.dev/doc/faq#nil_error interesting behavior of go with nil values and interfaces.

See this example https://go.dev/play/p/66QShXidalq

Basically we can't always compare result != nil, where result is an interface type. That might work or not depending on wazero's code. (it gets awkward when you remember that error is an interface).

checkError also looks at stderr, which make be written to if InstantiateModule does not return an error. Merged as is we wouldn't call result.Close in all cases.

checkError doesn't look at stderr when err is nil to begin with, so it should close in all relevant cases? But I agree the indirection makes things less clear.

I'm thinking we can just defer close when err == nil? Please see the new commit.