Proposal: Allow walk to run on a thread of the caller's choice

[JoshuaSjoding]

Walk currently requires that the message loop runs on the main thread—the thread that called init() and main(). While it is important that window creation and the message loop are thread-locked, running it on the main thread is not a requirement of Windows.

This requirement has led to issues like #297, #337 and #555. The workaround for #297 and #337 was to ensure that Run() took place on the main thread.

I propose making a few changes to allow window creation and Run() functions to work on a thread of the caller's choice.

Code like this should run and exit quickly:

func main() {
	var wg sync.WaitGroup
	wg.Add(1)
	go func() {
		defer wg.Done()
		mw, err := walk.NewMainWindow()
		if err != nil {
			panic(err)
		}
		defer mw.Dispose()
	}()
	wg.Wait()
}

Right now this code deadlocks. This is due to the current design for widget tool tip handling. I believe this can be fixed without breaking existing applications or changing walk's API.

Current Design

The existing tool tip implementation relies on a global instance of the ToolTip control called
globalToolTip. Widgets add themselves to and remove themselves from this global instance. The instance is prepared in an init() call (and has been since since 2012):

func init() {
	var err error
	if globalToolTip, err = NewToolTip(); err != nil {
		panic(err)
	}
}

Note that NewToolTip() calls newToolTip() which calls InitWindow(). This results in a CreateWindowEx() call on the main thread which results in the creation of a Windows message queue. Walk's tool tip implementation is the thing that's creating the Windows message queue and it's doing this in an init() call.

Widgets ultimately add and remove themselves by calling SendMessage() and sending TTM_ADDTOOL and TTM_DELTOOL messages to the globalToolTip control. SendMessage completely skips the message loop if the recipient was created by the calling thread, and walk relies on this fact to avoid deadlocks during initialization.

In the current design widgets must be added to a tool tip control on the same thread or they'll deadlock on SendMessage(). The globalToolTip is created on the main thread, therefore all widgets must also be created on the main thread.

Proposal Objective

Only create a ToolTip instance when a call to InitWindow() happens for some other reason first. This should be the result of the caller taking some explicit action like calling Run(), NewDialog() or NewMainWindow(). The caller makes the call from a goroutine of his or her choice. That's the thread where the message loop has to live so that's where the tool tip control should live too.

Proposed Design A

Instead of a global ToolTip control, keep a pointer to a local ToolTip instance in
FormBase or ContainerBase. Functions like NewMainWindow() would set this value when they prepare a window or form that has no parent.

Widgets add themselves to and and remove themselves from the ToolTip of their parent (or closest ancestor with a non-nil ToolTip).

Concerns

The SetParent() and SetOwner() functions pose a challenge. A widget could be re-parented to a window with a different tool tip control. Removing the tool from the old parent's control and adding it to the new parent's control might be sufficient.

Walking up the window hierarchy to locate an appropriate parent adds complexity. Every interaction with the local tool tip has an edge case where the ToolTip may not exist.

Status

Proposed Design A was not merged.

Proposed Design B

Create a new per-thread storage structure called a WindowGroup. Each Window will have a WindowGroup assigned to it as part of its InitWindow() process. All windows created on a common thread will share the same window group.

Widgets will access their ToolTip controls through their window group. The tool tip control is created on first use and is destroyed when the group is destroyed.

The life cycle of a window group is managed by reference counting. Window groups are destroyed once all windows that refer to the group are destroyed. The reference is incremented by InitWindow() and decremented by WindowBase.Dispose().

The current thread can be identified by a call to GetCurrentThreadId().

Concerns

Assessing the liveness of a window group relies on reference counting. If we miscount we could dispose of the tool tip prematurely or leak system resources.

Status

Proposed Design B was merged in #610.

Pros

Callers can create windows and run message loops in a goroutine of their choice.

These designs could pave the way for concurrent Run() invocations on different threads, so long as the caller doesn't pass widgets between threads. The runSynchronized() function also needs to stop relying on global state. That can go in a separate issue.

Cons

Callers must still be careful to create and run a related set of windows within a single thread.

[lxn]

Nice writeup!

Have you tried this with multiple forms?

What about stack size? Isn't the main goroutine the only one guaranteed to run on a thread with a nice big stack?

Would it be possible to prevent re-parenting from one thread to another? Windows should already do that I assume?

This probably is another issue with concurrent Run() calls. At least Application.activeForm would have to be protected with a mutex.

[JoshuaSjoding]

Have you tried this with multiple forms?

This code spawns 5 windows and causes 5 message loops to run concurrently:

	const count = 5
	var wg sync.WaitGroup
	wg.Add(count)

	spawn := func(i int) {
		defer wg.Done()

		var mw *walk.MainWindow

		go func() {
			time.Sleep(count * time.Second)
			mw.Synchronize(func() {
				mw.Close()
			})
		}()

		if _, err := (declarative.MainWindow{
			AssignTo: &mw,
			Title:    fmt.Sprintf("Window %d", i),
			MinSize:  declarative.Size{320, 240},
			Size:     declarative.Size{800, 600},
			Layout:   declarative.VBox{MarginsZero: true},
			Children: []declarative.Widget{
				declarative.Slider{
					MinValue:    0,
					MaxValue:    count,
					Value:       i,
					Orientation: declarative.Horizontal,
				},
			},
		}).Run(); err != nil {
			log.Fatal(err)
		}
	}

	for i := 0; i < count; i++ {
		if i > 0 {
			time.Sleep(time.Second)
		}
		go spawn(i)
	}

	wg.Wait()

When running against my feature branch each window has a functional message loop—the sliders respond to user input and to the close button. It's not an exhaustive test and I'm sure it races with Application.activeForm but it shows potential.

What about stack size? Isn't the main goroutine the only one guaranteed to run on a thread with a nice big stack?

Hrm, I hadn't considered that. I looked at #261 and #290 and I assume you're talking about the expectation some controls have about the stack size that's available to them, in particular the WebView control. More investigation is warranted.

Would it be possible to prevent re-parenting from one thread to another? Windows should already do that I assume?

We could use GetCurrentThreadId to store the creator thread ID in WindowBase as part of the InitWindow setup process. Then functions like SetParent could compare the thread IDs of the respective windows and complain or panic if they belong to different threads. In fact, this would open up all sorts of opportunities to catch mistakes and let the caller know they've done something wrong.

This probably is another issue with concurrent Run() calls. At least Application.activeForm would have to be protected with a mutex.

I hadn't noticed the appSingleton and App() calls yet. We'll need some sort of solution for that; guarding its data with a mutex sounds like a good start.

[JoshuaSjoding]

Proposed Design B has been merged and I think it's working well. I'm going to close this issue and open a separate one for running on multiple threads.