In-process GuiMacro Call Freezes for One Minute on Startup

[hypersw]

If I invoke a GuiMacro for GetInfo Root immediately after spawning ConEmu process, it freezes for about a minute before returning control.

Subsequent calls return as expected.

[hypersw]

After that long wait it yields CERR_GUIMACRO_FAILED.

Ain't got PBDs so can't know the native stack traces.

Might possibly be that code which waits for the conemu process to complete its init?.. In the in-process case, if completing the init means interacting with HWND, then it will block itself because HWND business requires the message pump running, but that thread is blocked with the wait instead.

[Maximus5]

How about latest versions?

[hypersw]

Waits for one minute, yields CERR_GUIMACRO_FAILED for the first call. Next calls after pumping the windows message queue are OK. Once it also crashed with a message similar to #4 after the wait, but I could not reproduce it again.

[hypersw]

The mode in the demo is switched with ConEmuSession::IsExecutingGuiMacrosInProcess.

[hypersw]

I've debugged it down to this location:

                    DWORD nWait, nDelay = hWaitProcess ? 60000 : 10000;
                    // Until succeeded?
                    do
                    {
                        // Wait a little while ConEmu is initializing
                        if (hWaitProcess)
                        {
                            nWait = WaitForSingleObject(hWaitProcess, 100);
                            if (nWait == WAIT_OBJECT_0)
                                break; // ConEmu.exe was terminated
                        }
                        else
                        {
                            Sleep(100);
                        }
                        // Recheck
                        if ((Inst.hConEmuWnd = GetConEmuExeHWND(Inst.nPID)) != NULL)
                            break; // Found
                    } while ((GetTickCount() - nStartTick) <= nDelay);

I've changed it to be:

                    DWORD nDelay = hWaitProcess ? 60000 : 10000;
                    // Select if we should pump the message queue while waiting — that's a convention for STA apartment threads
                    // A special case is Con Emu Inside HWND, which won't allow conemu complete the init process we're waiting for if we block the messages
                    // Detect if we have to pump by checking if the thread has a message pump already (and once you got it, you must pump it, that's the rule of thumb)
                    GUITHREADINFO dummyinfo = {0};
                    BOOL isThreadWithMessagePump = GetGUIThreadInfo(GetCurrentThreadId(), &dummyinfo); // Fails on threads with no message pump
                    // Until succeeded?
                    do
                    {
                        // Do the wait
                        // If we know the process, then wait for it; otherwise, just wait for a WM (if possible in this thread), or up until the timeout
                        DWORD nWait = MsgWaitForMultipleObjects((hWaitProcess?1:0), (hWaitProcess?&hWaitProcess:NULL), FALSE, 100, QS_ALLINPUT);
                        if(nWait == WAIT_OBJECT_0)
                            break; // ConEmu.exe was terminated

                        // Do the pumping if thread got queue
                        if (isThreadWithMessagePump)
                        {
                            MSG msg = { 0 };
                            while (PeekMessageW(&msg, NULL, 0, 0, PM_REMOVE) != 0)
                            {
                                TranslateMessage(&msg);
                                DispatchMessageW(&msg);
                            }
                        }

                        // Recheck
                        if ((Inst.hConEmuWnd = GetConEmuExeHWND(Inst.nPID)) != NULL)
                            break; // Found
                    } while ((GetTickCount() - nStartTick) <= nDelay);

So now the wait itself succeeded, but it ended even nastier because it got stuck in MFileMapping::InternalOpenCreate this time. Looks like it's not possible to do this synchronously on the same thread, because one or the other thing will either deadlock or encounter an inconsistent state.

There are two alternatives.

1. As we've gotten an HWND already, why not communicate over it? I remember some old software doing automation this way. Register a custom Windows Message by name, create a global string, do a SendMessage with that string in the lParam, receive another string in lResult. Or NULL, if ConEmu is not yet ready and is not processing messages on its HWND. That looks like a more appealing approach because it uses basic OS means over the object we've already inevitably gotten (the child HWND of ConEmu) and does not involve any manual manipulation with DLLs.
2. Threading play. Make another thread for comm, run a message pump on the calling STA thread while the comm thread is waiting or working. Sounds more complicated than 1.

[Maximus5]

How do you trigger this issue? Using ConsoleUtilityShowcase or anything else?

[Maximus5]

And what do you mean by "global string"?

Anyway, I do not want to post Macros into ConEmu main thread, because

• they were intended to be asynchronous
• SendMessage fails in some circumstances (right access)

[hypersw]

Unfortunately, no. It's a kind of an end-user example which uses two nugets as input, and Visual Studio does not quite support this as a simple scenario for local debug: can't easily make one project output into a nuget input for another. I've patched the original main exe to reference the control as a VS project (even before there were showcase project and nugets). See in my fork's master branch. Maybe I should be rather doing that in a feature branch in the main conemu-inside repo? Would be easier to sync and debug.

[hypersw]

I believe access shouldn't be a problem when we're hosting an HWND of a child process in our own already, but the threading issue is much reasonable.

Loading a DLL isn't so bad, though there're two issues:

1. I fear it won't work reliably on the main thread. At least it can go into a sync wait (at a glance there's a number of such places in this dll), which isn't nice for a GUI app, not to mention the deadlocks. For this, I'd use Task1` and emulate sync execution for the main thread. Will try out in a day, so you can postpone hardcore debugging in case it proves workable.
2. It uses process-global static context for passing the return value. This smells bad, and then adding threads makes this look a disaster. I'd vote for using some contract on strings to alloc by caller and free by callee. That's not even interprocess as we only talk to a loaded DLL here.

[Maximus5]

I can easily implement macro execution in the separate thread and result return via sort of callback. But how this callback may be triggered in the .Net application? I'm not sure if it is possible to pass .net function pointer to native code.

[Maximus5]

As an option, we may use pending technique. Application creates a request, dll returns an id, application poll for result using this id, finally application closes the request (releasing allocated buffers).

[Maximus5]

Managed to reproduce deadlock in guimacro branch. Must be fixed in ConEmu.

[Maximus5]

I've created winforms branch and you may push right here.

[Maximus5]

Deadlock problem solved in master: f4534b6, 5cc91ae

[hypersw]

.NET can do well with native thunks to managed code. Most fluently with COM interfaces — here I don't mean OLE or Registry or marshalling, proxies, etc — just the direct C++ calls stuff plus conventions on calling model, data layout, functions, ownership transfer and so on. But that means some infrastructure around, though it's easy to understand what to expect from it. Another option is to just make a native thunk for a managed function via Marshal::GetFunctionPointerForDelegate. Its memory management has to be done manually though. So the callback must be quite reliable to avoid memory leaks.

Except for COM which has a strong story of ownership transfer and memory release, all this stuff is no more reliable than calling SysAllocString in the macro handler, and making caller do SysFreeString by contract. Or maybe VirtualAlloc if you don't like the former functions. The callback or request protocol isn't more reliable or mem-leak-safe. Especially around scenarios like killing conemu or disposing of the control in the middle of an async request.

[Maximus5]

I think this issue may be closed. Take a look at new interface.

[hypersw]

The current implementation would crash eventually as the fnCallback thunk might be reclaimed by garbage collection before the DLL calls it back. Some additional memory management would be required, depending on when the callback might be invoked — what's the contract here, is it definitely within the call, or possibly async?

All in all this does not look simpler than SysFreeString (or GlobalFree) on part of the caller, just curious as of the reasons why that option were turned down.

[hypersw]

I've adopted the new function, works nicely with a locally-compiled conemu. As .14 nuget is published, would work out of the box.

[Maximus5]

Please look at new prototype. Indeed, using BSTR is more reliable that callback, because GuiMacro function is synchronous.

[hypersw]

For all that I know, this prototype should do with fully automatic resource reclamation, as the pattern now exactly follows the COM spec for ownership transfer:

[UnmanagedFunctionPointer(CallingConvention.StdCall, CharSet = CharSet.Unicode)]
public delegate int FGuiMacro([MarshalAs(UnmanagedType.LPWStr)] string asInstance, [MarshalAs(UnmanagedType.LPWStr)] string asMacro, [MarshalAs(UnmanagedType.BStr)] out string bsResult);

But I'm not sure how to check this other than debugger-step through native assembly. But I'd be much surprised if it's not OK.