StubExecutable crashes on update / uninstall

[Glebka]

Squirrel version(s)
1.9.1

Description
AppName_ExecutionStub.exe crashes during autoupdate / uninstall of our Electron app.
More details in Additional information section.

Steps to recreate
Trigger update of Electron App from older to newer version.

Expected behavior
AppName_ExecutionStub.exe should not crash. It should exit with some error code, for example -1.

Actual behavior
AppName_ExecutionStub.exe crashes with core dump generation.

Additional information

This happens because installer treats AppName_ExecutionStub.exe as a Squirrel Aware app and starts executing hooks on it (calling it with additional command line arguments such as "--squirrel-updated 19.6.1-beta").
Well, actually, AppName_ExecutionStub.exe is Squirrel Aware
because during the release build process Squirrel copies resources from the main app to the StubExecutable.

Squirrel.Windows/src/Update/Program.cs

Line 634 in bdfcd72

String.Format("--copy-stub-resources \"{0}\" \"{1}\"", fullName, target),

When StubExecutable starts, it tries to find the main executable and run it with passed arguments. In this particular case installer calls StubExecutable that is already located in AppData/Local/AppName/app-VERSION, thus it fails to find main executable.
I collected a core dump of the StubExecutable. So here some variable values ad call stack that reflects the state of executable when it crashed:

Variables

	Name	Value	Type
▶	&fileInfo	0x004efa68 L"" 0x00000000	_WIN32_FIND_DATAW *
▶	fileInfo	L"" 0x00000000	_WIN32_FIND_DATAW
	hFile	0xffffffff	void *
▶	ourDir	L"C:\Users\vagrant\AppData\Local\AppName\app-19.6.1-beta\app-*"	std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >
▶	cmdLine	0x008b19bc L""C:\Users\vagrant\AppData\Local\AppName\app-19.6.1-beta\AppName_ExecutionStub.exe" --squirrel-updated 19.6.1-beta"	wchar_t *

Call stack

 	AppName_ExecutionStub.exe!std::char_traits<wchar_t>::length(const wchar_t * _First) Line 303	C++
 	[Inline Frame] AppName_ExecutionStub.exe!std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >::assign(const wchar_t *) Line 1182	C++
 	AppName_ExecutionStub.exe!std::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >::basic_string<wchar_t,std::char_traits<wchar_t>,std::allocator<wchar_t> >(const wchar_t * _Ptr) Line 838	C++
>	AppName_ExecutionStub.exe!FindLatestAppDir() Line 53	C++
 	AppName_ExecutionStub.exe!wWinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, wchar_t * lpCmdLine, int nCmdShow) Line 97	C++
 	[External Code]	
 	[Frames below may be incorrect and/or missing, no symbols loaded for kernel32.dll]	Unknown

I found here

Squirrel.Windows/src/StubExecutable/StubExecutable.cpp

Line 43 in bdfcd72

std::wstring FindLatestAppDir()

that FindLatestAppDir() may return NULL in some cases, but defined return type is std::wstring. That is the root cause of the crash. I think we need to replace NULL with empty std::wstring. I can create a PR later for this.

[Thieum]

@Glebka Nice research! I'm sure a PR would be much appreciated.

[Glebka]

@Thieum I've posted a PR, please review

[Thieum]

@Glebka I think only @shiftkey and @robmen have that power atm.

[bddckr]

This reminds me of #1355 - perhaps it's related?

[anaisbetts]

I think, that given that we understand the underlying issue, this fix could be improved, we're kind of paving over the actual issue instead of just fixing the cause. The correct fix is either:

• Squirrel should not mark stub executables as Squirrel-Aware during the resource copy step (hard, because we don't want to get into the game of stripping attributes from RC files and assemblies)

• Squirrel should detect an executable (probably by name) as a stub, and ignore it for the purposes of running down Squirrel hooks (i.e. "Stubs can never be Squirrel-aware"). This is an easy fix and is More Correct