Add Func/FuncOf

[hajimehoshi]

This change adds Func and FuncOf, which will be introduced at Go 1.12 to replace Callback. GopherWasm doesn't remove Callback but keep them for backward compatibility by emulating them with Func.

With Go 1.11 and Wasm, Func doesn't work synchronously.

Fixes #21

[hajimehoshi]

I don't understand what TravisCI complains...

9.47s$ GO111MODULE=off ~/go1.11.4/bin/go get github.com/gopherjs/gopherjs/...
# errors
flag provided but not defined: -std

[hajimehoshi]

TravisCI's error is not this PR's fault:

https://travis-ci.org/gopherjs/gopherwasm/builds/474976371

Now even master branch fails.

This was my misunderstanding, ignore this.

[hajimehoshi]

Please take a look @neelance @dmitshur

One of the two errors on TravisCI is due to the difference of $TRAVIS_BRANCH, so we don't have to care.

[neelance]

Are you sure it's a good idea that this allows the situation where code works fine with Go 1.12, but fails with Go 1.11 even though it compiles fine? Maybe it would be better to only allow Go 1.11 code to run on 1.12, but not the other way around. That's what Go usually does.

[hajimehoshi]

Right. Go 1.11's Callback cannot emulate Func correctly on Wasm. I believe both can be emulated on GopherJS though. I thought the advantage of preparing Func even for Go 1.11 was that the developers could prepare for new Func from now on, but as this could not be emulated correctly, this might not make sense.

Do you think it'd be fine to give up to emulate Func on Go 1.11 and allow Func only on Go 1.12 and later (and GopherJS)? This means that gopherwasm.Func will be defined only when Go 1.12 or later (or GopherJS) is used.

[neelance]

Do you think it'd be fine to give up to emulate Func on Go 1.11 and allow Func only on Go 1.12 and later (and GopherJS)? This means that gopherwasm.Func will be defined only when Go 1.12 or later (or GopherJS) is used.

Yes, I think this would be safer.

[hajimehoshi]

Done.

[hajimehoshi]

I found a deadlock error on gopherjs test. Hmm

fatal error: all goroutines are asleep - deadlock!

[hajimehoshi]

@neelance Would you give me an advice about the deadlock? If we couldn't solve this, we would need to use the previous way instead of unified FuncOf way.

[neelance]

I think if you use the previous way you would just hide the bug (test case not covering it) instead of solving it. Using FuncOf is correct on a theoretical level and if it works on one platform, you should compare the behavior of both platforms to figure out the difference.

[hajimehoshi]

I think if you use the previous way you would just hide the bug (test case not covering it) instead of solving it.

I agree, but unfortunately, in the current state of GopherJS community, fixing and committing this bug would take long time. I'll try to confirm whether the cause is in GopherJS or not though.

[hajimehoshi]

I forgot I run into the same problem before...! https://github.com/gopherjs/gopherjs/issues/826

[neelance]

I don't know, it is already some time ago that I last worked on GopherJS.

[hajimehoshi]

OK my understanding is:

• MakeFunc does nothing wrong.
• Blocking the main goroutine (e.g. by reading a channel) is forbidden in GopherJS, and this has nothing to do with MakeFunc. I'm not sure the reason, but MakeFunc exposes this problem.
• gopherjs test still warns about deadlock (even when running no tests!). As the tests pass, I'd like to revisit this problem later. Usual (and correct) usage does not cause the deadlock.
• Ebiten blocks the main thread (e.g. Run never returns), which is not a good manner as I described, and MakeFunc exposes this problem. That's unfortunate. I probably would need to compromise not to block the main goroutine.

Then I'm OK to use MakeFunc way. Could you take a look? Thanks!

[hajimehoshi]

ping

[neelance]

Blocking the main goroutine (e.g. by reading a channel) is forbidden in GopherJS

This is not true. It's only forbidden to block in a callback, see https://github.com/gopherjs/gopherjs#goroutines.

[Noofbiz]

I actually ran into this issue when I was removing the audio system from the main thread in engo. It was coming from this function. It ended up being fixed by adding the return statement at the end there. I'm still not sure why lol. Hope this can help!

[hajimehoshi]

Thanks! In my case, leaving return didn't help unfortunately.

https://github.com/hajimehoshi/ebiten/blob/690dae0488c7bb339c66ed1295903a8bd4d43dbc/internal/ui/ui_js.go#L238

It looks like the deadlock warning happens in the loop, but I don't know.

[hajimehoshi]

@neelance So, apart from this 'dead-lock' behavior of GopherJS (I think this is a bug), do you think it is fine to merge this PR? Thanks!

[hajimehoshi]

Thank you!

[hajimehoshi]

That's odd, TravisCI now fails :thinking: Should we revert this change?

[neelance]

Your project, your decision. ;-)

[hajimehoshi]

https://github.com/gopherjs/gopherwasm/pull/23 fixed this issue 😄

[neelance]

Perfect!