From d1958fa5cf9bdbf27d488865e7d6744aee10f555 Mon Sep 17 00:00:00 2001 From: David Crawshaw Date: Sat, 27 Jun 2015 18:32:29 -0400 Subject: [PATCH] app: remove error return from Main Move runtime.LockOSThread from the portable Main to the platform-specific android main. For darwin, calling LockOSThread in Main is too late, there has been enough time for the goroutine calling it to jump off the initial OS thread. For darwin we call LockOSThread from an init function, which the runtime keeps on the first thread. For Android we call LockOSThread only to maintain the thread-local storage for the GL context that is created later, so it is safe to call it from func main. Also remove TODOs about starting a gobind app on Android, which will be simplified in a followup CL. Tested on darwin/amd64 and android. Not tested on X11. Change-Id: I34c56abf8b1292959d4d508bfade287d196c0380 Reviewed-on: https://go-review.googlesource.com/11653 Reviewed-by: Nigel Tao --- app/android.go | 38 ++++++++++++++------------------------ app/app.go | 12 +++--------- app/darwin_amd64.go | 6 ++---- app/loop_android.go | 8 ++++---- app/x11.go | 13 ++++++++----- 5 files changed, 31 insertions(+), 46 deletions(-) diff --git a/app/android.go b/app/android.go index 80482b6..40217f1 100644 --- a/app/android.go +++ b/app/android.go @@ -63,6 +63,7 @@ import ( "io" "log" "os" + "runtime" "time" "unsafe" @@ -274,40 +275,29 @@ func (a *asset) Close() error { return nil } -// TODO(crawshaw): fix up this comment?? -// notifyInitDone informs Java that the program is initialized. -// A NativeActivity will not create a window until this is called. -func main(f func(App) error) error { +func main(f func(App)) { + // Preserve this OS thread for the GL context created in windowDraw. + runtime.LockOSThread() + ctag := C.CString("Go") cstr := C.CString("app.Run") C.__android_log_write(C.ANDROID_LOG_INFO, ctag, cstr) C.free(unsafe.Pointer(ctag)) C.free(unsafe.Pointer(cstr)) - donec := make(chan error, 1) + donec := make(chan struct{}) go func() { - donec <- f(app{}) + f(app{}) + close(donec) }() if C.current_native_activity == nil { - // TODO: Even though c-shared mode doesn't require main to be called - // now, gobind relies on the main being called. In main, app.Run is - // called and the start callback initializes Java-Go communication. - // - // The problem is if the main exits (because app.Run returns), go - // runtime exits and kills the app. - // - // Many things have changed in cgo recently. If we can manage to split - // gobind app, native Go app initialization logic, we may able to - // consider gobind app not to use main of the go package. - // - // TODO: do we need to do what used to be stateStart or stateStop? - select {} - } else { - for w := range windowCreated { - if done, err := windowDraw(w, queue, donec); done { - return err - } + <-donec + return + } + for w := range windowCreated { + if windowDraw(w, queue, donec) { + return } } panic("unreachable") diff --git a/app/app.go b/app/app.go index a849242..6ad3d70 100644 --- a/app/app.go +++ b/app/app.go @@ -8,8 +8,6 @@ package app import ( "io" - "log" - "runtime" "golang.org/x/mobile/event" ) @@ -18,11 +16,8 @@ import ( // // It calls f on the App, in a separate goroutine, as some OS-specific // libraries require being on 'the main thread'. -func Main(f func(App) error) { - runtime.LockOSThread() - if err := main(f); err != nil { - log.Fatal(err) - } +func Main(f func(App)) { + main(f) } // App is how a GUI mobile application interacts with the OS. @@ -152,7 +147,7 @@ func pump(dst chan interface{}) (src chan interface{}) { // // Deprecated: call Main directly instead. func Run(cb Callbacks) { - Main(func(a App) error { + Main(func(a App) { var c event.Config for e := range a.Events() { switch e := event.Filter(e).(type) { @@ -183,7 +178,6 @@ func Run(cb Callbacks) { } } } - return nil }) } diff --git a/app/darwin_amd64.go b/app/darwin_amd64.go index 551ace3..a6cce31 100644 --- a/app/darwin_amd64.go +++ b/app/darwin_amd64.go @@ -51,19 +51,17 @@ func init() { initThreadID = uint64(C.threadID()) } -func main(f func(App) error) error { +func main(f func(App)) { if tid := uint64(C.threadID()); tid != initThreadID { log.Fatalf("app.Main called on thread %d, but app.init ran on %d", tid, initThreadID) } - var err error go func() { - err = f(app{}) + f(app{}) // TODO(crawshaw): trigger runApp to return }() C.runApp() - return err } var windowHeight geom.Pt diff --git a/app/loop_android.go b/app/loop_android.go index a850bed..de0ef11 100644 --- a/app/loop_android.go +++ b/app/loop_android.go @@ -92,7 +92,7 @@ import ( var firstWindowDraw = true -func windowDraw(w *C.ANativeWindow, queue *C.AInputQueue, donec chan error) (done bool, err error) { +func windowDraw(w *C.ANativeWindow, queue *C.AInputQueue, donec chan struct{}) (done bool) { C.createEGLWindow(w) // TODO: is the library or the app responsible for clearing the buffers? @@ -136,8 +136,8 @@ func windowDraw(w *C.ANativeWindow, queue *C.AInputQueue, donec chan error) (don for { processEvents(queue) select { - case err := <-donec: - return true, err + case <-donec: + return true case <-windowRedrawNeeded: // Re-query the width and height. C.querySurfaceWidthAndHeight() @@ -170,7 +170,7 @@ func windowDraw(w *C.ANativeWindow, queue *C.AInputQueue, donec chan error) (don } case <-windowDestroyed: sendLifecycle(event.LifecycleStageAlive) - return false, nil + return false case <-gl.WorkAvailable: gl.DoWork() case <-endDraw: diff --git a/app/x11.go b/app/x11.go index f840ec6..4777718 100644 --- a/app/x11.go +++ b/app/x11.go @@ -24,6 +24,7 @@ void swapBuffers(void); */ import "C" import ( + "runtime" "time" "golang.org/x/mobile/event" @@ -31,15 +32,17 @@ import ( "golang.org/x/mobile/gl" ) -func main(f func(App) error) error { +func main(f func(App)) { + runtime.LockOSThread() C.createWindow() // TODO: send lifecycle events when e.g. the X11 window is iconified or moved off-screen. sendLifecycle(event.LifecycleStageFocused) - donec := make(chan error, 1) + donec := make(chan struct{}) go func() { - donec <- f(app{}) + f(app{}) + close(donec) }() // TODO: can we get the actual vsync signal? @@ -49,8 +52,8 @@ func main(f func(App) error) error { for { select { - case err := <-donec: - return err + case <-donec: + return case <-gl.WorkAvailable: gl.DoWork() case <-endDraw: