search

conda / constructor

tool for creating installers from conda packages
https://conda.github.io/constructor/
Other
460 stars 167 forks source link

[Windows] Python Installer crashed using nsis 3.08 #526

Closed JinYongWu closed 2 years ago

JinYongWu commented 2 years ago

Checklist

What happened?

We created a customized Python toolkit installer using constructor, the installer worked well when using nsis 3.01. Recently we upgraded nsis to 3.08 which provides the capability to sign the uninstaller exe. After switched to nsis 3.08(with no any other additional changes), we experienced crashes during installation process. Here are the high level steps to observe the crashes:

  1. Generate the customized Python toolkit installer using constructor (and nsis 3.08);

  2. Install the generated installer for the first time, noticed that everything is installed correctly;

  3. Uninstall the Python toolkit from "Add & Remove programs";

  4. Run the installer again;

  5. This time the installation window disappeared after a while, and checked the installation folder, there were just a few folders and the packages were not extracted, the subfolders and files are as below:

    conda-meta [folder]
Lib [folder]
pkgs [folder]
_conda.exe [file]

  6. Checked the task manager, noticed that there are 3 conda processes running under Background processes - even the installation window disappeared;

  7. Checked the windows Event Viewer, noticed that there were some faulting errors for the mentioned installer:

    Faulting application name: PythonToolkitInstallTool.exe, version: 2022.1.0.0, time stamp: 0x614f9b5a
Faulting module name: nsExec.dll, version: 0.0.0.0, time stamp: 0x614f9a06
Exception code: 0xc0000005
Fault offset: 0x00001564
Faulting process id: 0x1ac
Faulting application start time: 0x01d88c6c9f7ae701
Faulting application path: C:\Distr\PythonToolkitInstallTool.exe
Faulting module path: C:\Users\demo\AppData\Local\Temp\nsh4DF0.tmp\nsExec.dll
Report Id: b1533623-83c7-48e8-9778-48d2f9aad5be
Faulting package full name: 
Faulting package-relative application ID:

and information report:

Fault bucket 1397276649508340299, type 1
Event Name: APPCRASH
Response: Not available
Cab Id: 0

Problem signature:
P1: PythonToolkitInstallTool.exe
P2: 2022.1.0.0
P3: 614f9b5a
P4: nsExec.dll
P5: 0.0.0.0
P6: 614f9a06
P7: c0000005
P8: 00001564
P9: 
P10: 

Attached files:
\\?\C:\ProgramData\Microsoft\Windows\WER\Temp\WERCDE8.tmp.dmp
\\?\C:\ProgramData\Microsoft\Windows\WER\Temp\WERCFBE.tmp.WERInternalMetadata.xml
\\?\C:\ProgramData\Microsoft\Windows\WER\Temp\WERD06B.tmp.xml
\\?\C:\ProgramData\Microsoft\Windows\WER\Temp\WERD078.tmp.csv
\\?\C:\ProgramData\Microsoft\Windows\WER\Temp\WERD0A8.tmp.txt

These files may be available here:
\\?\C:\ProgramData\Microsoft\Windows\WER\ReportArchive\AppCrash_PythonToolkitIns_9f5d981163b5c43653c0d5beb4183a411697a096_7e09f96b_feb4c539-e392-41b1-972a-a430b1dafb39

Analysis symbol: 
Rechecking for solution: 0
Report Id: b1533623-83c7-48e8-9778-48d2f9aad5be
Report Status: 268435456
Hashed bucket: 7211a04ea917b3d7a3641fb5aae4824b
Cab Guid: 0

If we only change the nsis to version 3.01, the installer generated with same configuration worked fine for above steps. We compared and made sure that the issue happened as soon as we only upgrade nsis package to 3.08.

Conda Info

No response

Conda Config

No response

Conda list

No response

Additional Context

No response

scottwsides commented 2 years ago

I havent checked nsis version... but my windows conda installers are intermittently failing for the last couple of weeks. I always use the latest and greatest conda constructor version on windows.

jaimergp commented 2 years ago

I am seeing some issues in the CI too, where some Windows jobs suddenly hang indefinitely. Python version doesn't seem to be a factor.

jaimergp commented 2 years ago

First time it hang was May 11th (commit be1b1f658d73b86835323e11a3dfb5d604495ee0), just one day after nsis 3.08 was made available on defaults.

So yes, it looks like there are some sporadic problems where the installer might not run successfully (but sometimes it does?). I'd recommend pinning nsis to 3.01 in the meantime.

jaimergp commented 2 years ago

Maybe related: https://github.com/actions/virtual-environments/issues/4739#issuecomment-999781223

scottwsides commented 2 years ago

Yeah... that timeframe sounds about right. I figured out the problem I was seeing. I was substituting a custom default user path for windows and it was missing a trailing '\'. This reliably causes the 'intermittent' behavior (I know that sounds weird). This also explains when my users would type in an install path and leave out the trailing slash. If you use the menu to create a directory then it substitutes the backslash... but if you type in manually you can break it. I'm not super familiar with NSIS but this sounds like something it could/should check for and compensate. The really bad thing is... that if that trailing slash isn't there... no error message gets printed... the installer hard crashes. Took me a LONG time to figure out what was going on.

jlstevens commented 2 years ago

I'd recommend pinning nsis to 3.01 in the meantime.

This is good advice for people who don't need any of the new NSIS features though some users may need things like the
!uninstfinalize command which is only available in >3.01.

I'll also add that I couldn't get a windows installer to complete without hanging when built against 3.08 while the installer built against 3.01 worked just fine (just to confirm the issue as described above).

JinYongWu commented 2 years ago

Maybe related: actions/virtual-environments#4739 (comment)

I also tried to put "Unicode false" at the beginning of nsis script to use NSIS 3.08. I got similar errors caused by that some plugins were not loaded, i.e. UAC.dll.

jaimergp commented 2 years ago

For now I am going to issue some repodata patches so constructor only uses NSIS 3.01. Next release will hopefully address the problem.

jaimergp commented 2 years ago

I am taking another look at this issue and I can't manage to reproduce the problem in my VM. I am trying to use NSIS log builds to try and catch where the error is coming from, so the fact that I don't run into this issue with the log builds means that it could be a packaging problem?

Let's see if this fixes your problem temporarily, @JinYongWu:

  1. Create a new constructor environment with conda create -n constructor-log-builds constructor nsis=3.08 --copy. We need 'copy' so we can overwrite stuff safely.
  2. Download the NSIS log builds from here.
  3. Extract the ZIP and overwrite the contents of %CONDA_PREFIX%\NSIS with the contents of the ZIP.
  4. Run constructor path/to/construct.yml as usual.
jaimergp commented 2 years ago

Other things to try:

Diff for `nsexec.c` between versions ```diff --- /Users/jrodriguez/Downloads/nsis-3.01-src/Contrib/nsExec/nsexec.c 2022-09-05 12:36:55.000000000 +0200 +++ /Users/jrodriguez/Downloads/nsis-3.08-src/Contrib/nsExec/nsexec.c 2022-09-05 12:36:55.000000000 +0200 @@ -1,5 +1,6 @@ /* -Copyright (c) 2002 Robert Rainwater +Copyright (C) 2002 Robert Rainwater +Copyright (C) 2002-2021 Nullsoft and Contributors This software is provided 'as-is', without any express or implied warranty. In no event will the authors be held liable for any damages @@ -17,8 +18,6 @@ misrepresented as being the original software. 3. This notice may not be removed or altered from any source distribution. -Unicode support by Jim Park -- 08/24/2007 - */ #include #include @@ -32,114 +31,166 @@ #endif //~ _MSC_VER >= 1500 #endif //~ _MSC_VER -#ifndef true -#define true TRUE -#endif -#ifndef false -#define false FALSE -#endif +#define TAB_REPLACE _T(" ") +#define TAB_REPLACE_SIZE (sizeof(TAB_REPLACE) - sizeof(_T(""))) +#define TAB_REPLACE_CCH (TAB_REPLACE_SIZE / sizeof(_T(""))) +enum { MODE_IGNOREOUTPUT = 0, MODE_LINES = 1, MODE_STACK = 2 }; #define LOOPTIMEOUT 100 HWND g_hwndParent; HWND g_hwndList; +HINSTANCE g_hInst; void ExecScript(BOOL log); -void LogMessage(const TCHAR *pStr, BOOL bOEM); TCHAR *my_strstr(TCHAR *a, TCHAR *b); unsigned int my_atoi(TCHAR *s); int WINAPI AsExeWinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPTSTR lpCmdLine, int nCmdShow); void __declspec(dllexport) Exec(HWND hwndParent, int string_size, TCHAR *variables, stack_t **stacktop) { - g_hwndParent=hwndParent; + g_hwndParent = hwndParent; EXDLL_INIT(); - { - ExecScript(0); - } + ExecScript(MODE_IGNOREOUTPUT); } void __declspec(dllexport) ExecToLog(HWND hwndParent, int string_size, TCHAR *variables, stack_t **stacktop) { - g_hwndParent=hwndParent; + g_hwndParent = hwndParent; EXDLL_INIT(); - { - ExecScript(1); - } + ExecScript(MODE_LINES); } void __declspec(dllexport) ExecToStack(HWND hwndParent, int string_size, TCHAR *variables, stack_t **stacktop) { - g_hwndParent=hwndParent; + g_hwndParent = hwndParent; EXDLL_INIT(); - { - ExecScript(2); - } + ExecScript(MODE_STACK); } -HINSTANCE g_hInst; BOOL WINAPI DllMain(HINSTANCE hInst, ULONG ul_reason_for_call, LPVOID lpReserved) { g_hInst = hInst; return TRUE; } -#define TAB_REPLACE _T(" ") -#define TAB_REPLACE_SIZE (sizeof(TAB_REPLACE)-1) +static BOOL IsLeadSurrogateUTF16(unsigned short c) { return c >= 0xd800 && c <= 0xdbff; } +static BOOL IsTrailSurrogateUTF16(unsigned short c) { return c >= 0xdc00 && c <= 0xdfff; } -BOOL IsWOW64() { - typedef BOOL (WINAPI *LPFN_ISWOW64PROCESS) (HANDLE, PBOOL); - BOOL wow64; - LPFN_ISWOW64PROCESS fnIsWow64Process; - - fnIsWow64Process = (LPFN_ISWOW64PROCESS) GetProcAddress( - GetModuleHandle(_T("kernel32")), "IsWow64Process"); - - if (fnIsWow64Process != NULL) { - if (fnIsWow64Process(GetCurrentProcess(), &wow64)) { - return wow64; +static PWSTR MyCharNext(PCWSTR p) +{ + // Note: This is wrong for surrogate pair combining characters but CharNextW does + // not support surrogate pairs correctly so we have to manually handle the pairs. + if (!p[0]) return (PWSTR) p; + if (IsLeadSurrogateUTF16(p[0]) && IsTrailSurrogateUTF16(p[1])) return (PWSTR) p + 2; // Current is a surrogate pair, we incorrectly assume that it is not followed by combining characters. + if (IsLeadSurrogateUTF16(p[1]) && IsTrailSurrogateUTF16(p[2])) return (PWSTR) p + 1; // Next is a surrogate pair, we incorrectly assume that it is not a combining character for the current character. + return (CharNextW)(p); +} +#define CharNextW MyCharNext + +static void TruncateStringUTF16LE(LPWSTR Buffer, SIZE_T Length, LPCWSTR Overflow, SIZE_T lenOver) { + if (Length) { + LPWSTR p = &Buffer[Length - 1]; + UINT stripBaseCharIfCuttingCombining = TRUE; + + // CharNextW is buggy on XP&2003 but we don't care enough to call GetStringTypeW (http://archives.miloush.net/michkap/archive/2005/01/30/363420.html) + if (stripBaseCharIfCuttingCombining && lenOver) { + WCHAR buf[] = { *p, Overflow[0], lenOver > 1 ? Overflow[1] : L' ', L'\0' }; + for (;;) { + BOOL comb = CharNextW(buf) > buf + 1; + if (!comb || p < Buffer) break; + *((WORD*)((BYTE*)&buf[1])) = *((WORD*)((BYTE*)&buf[0])); + buf[0] = *p; + *p-- = L'\0'; + } + } + + if (IsLeadSurrogateUTF16(*p)) { + *p = L'\0'; // Avoid incomplete pair } } +} + +static void TruncateStringMB(UINT Codepage, LPSTR Buffer, SIZE_T Length, unsigned short OverflowCh) { + if (Length) { + CHAR *p = &Buffer[Length - 1], buf[] = { *p, ' ', ' ', '\0' }; + if (CharNextExA(Codepage, buf, 0) > buf + 1) { // Remove incomplete DBCS character? + *p = '\0'; + } + } +} + +static BOOL IsWOW64() { +#ifdef _WIN64 return FALSE; +#else + typedef BOOL (WINAPI*ISWOW64PROCESS)(HANDLE, BOOL*); + ISWOW64PROCESS pfIsWow64Process; + typedef BOOL (WINAPI*ISWOW64PROCESS2)(HANDLE, USHORT*, USHORT*); + ISWOW64PROCESS2 pfIsWow64Process2; + HANDLE hProcess = GetCurrentProcess(); + HMODULE hK32 = GetModuleHandleA("KERNEL32"); + UINT_PTR retval; + USHORT appmach, image_file_machine_unknown = 0; + CHAR funcnam[16] +#if defined(_MSC_VER) && (_MSC_VER-0 <= 1400) + = "IsWow64Process2"; // MOVSD * 4 +#else + ; lstrcpyA(funcnam, "IsWow64Process2"); +#endif + pfIsWow64Process2 = (ISWOW64PROCESS2) GetProcAddress(hK32, funcnam); + if (pfIsWow64Process2 && pfIsWow64Process2(hProcess, &appmach, NULL)) { + retval = image_file_machine_unknown != appmach; + } + else { + BOOL wow64; + pfIsWow64Process = (ISWOW64PROCESS) GetProcAddress(hK32, (funcnam[14] = '\0', funcnam)); + retval = (UINT_PTR) pfIsWow64Process; + if (pfIsWow64Process && (retval = pfIsWow64Process(hProcess, &wow64))) { + retval = wow64; + } + } + return (BOOL) (UINT) retval; +#endif } -/** - * Convert the ansiStr if storing ANSI strings, otherwise, assume that the - * string is wide and don't convert, but straight copy. - * @param ansiStr [in] the suspected ANSI string. - * @param wideBuf [out] the buffer to write to. - * @param cnt [in] the size of widebuf in wchar_t's. - * @return true, if ASCII, false if suspected as wide. - */ -BOOL WideConvertIfASCII(const char* ansiStr, int strLen, WCHAR* wideBuf, int cnt) -{ - BOOL rval = FALSE; - wideBuf[0] = 0; - if (lstrlenA(ansiStr) == strLen) - { - MultiByteToWideChar(CP_ACP, 0, ansiStr, -1, wideBuf, cnt); - rval = TRUE; - } - else - { - // Going to assume that it's a wide char array. - lstrcpyW(wideBuf, (const WCHAR*) ansiStr); - } - - return rval; -} - -void ExecScript(int log) { - TCHAR szRet[128] = _T(""); - TCHAR meDLLPath[MAX_PATH]; - TCHAR *executor; - TCHAR *g_exec; +// Tim Kosse's LogMessage +#ifdef UNICODE +static void LogMessage(const TCHAR *pStr, BOOL bOEM) { +#else +static void LogMessage(TCHAR *pStr, BOOL bOEM) { +#endif + LVITEM item; + int nItemCount; + if (!g_hwndList) return; + //if (!*pStr) return; +#ifndef UNICODE + if (bOEM == TRUE) OemToCharBuff(pStr, pStr, lstrlen(pStr)); +#endif + nItemCount=(int) SendMessage(g_hwndList, LVM_GETITEMCOUNT, 0, 0); + item.mask=LVIF_TEXT; + item.pszText=(TCHAR *)pStr; + item.cchTextMax=0; + item.iItem=nItemCount, item.iSubItem=0; + ListView_InsertItem(g_hwndList, &item); + ListView_EnsureVisible(g_hwndList, item.iItem, 0); +} + + +void ExecScript(int mode) { + TCHAR szRet[128]; + TCHAR meDLLPath[MAX_PATH]; + TCHAR *g_exec, *executor; TCHAR *pExec; - unsigned int g_to; - BOOL bOEM; + int ignoreData = mode == MODE_IGNOREOUTPUT; + int logMode = mode == MODE_LINES, stackMode = mode == MODE_STACK; + unsigned int to, tabExpandLength = logMode ? TAB_REPLACE_CCH : 0, codepage; + BOOL bOEM, forceNarrowInput = FALSE; + + *szRet = _T('\0'); if (!IsWOW64()) { TCHAR* p; int nComSpecSize; nComSpecSize = GetModuleFileName(g_hInst, meDLLPath, MAX_PATH) + 2; // 2 chars for quotes - g_exec = (TCHAR *)GlobalAlloc(GPTR, sizeof(TCHAR)*(g_stringsize+nComSpecSize+2)); // 1 for space, 1 for null + g_exec = (TCHAR *)GlobalAlloc(GPTR, sizeof(TCHAR) * (g_stringsize+nComSpecSize+2)); // 1 for space, 1 for null p = meDLLPath + nComSpecSize - 2; // point p at null char of meDLLPath *g_exec = _T('"'); executor = g_exec + 1; @@ -198,18 +249,18 @@ pExec++; } else { executor = NULL; - g_exec = (TCHAR *)GlobalAlloc(GPTR, sizeof(TCHAR)*(g_stringsize+1)); // 1 for NULL + g_exec = (TCHAR *)GlobalAlloc(GPTR, sizeof(TCHAR) * (g_stringsize+1)); // 1 for NULL pExec = g_exec; } - g_to = 0; // default is no timeout - bOEM = FALSE; // default is no OEM->ANSI conversion + to = 0; // default is no timeout + bOEM = FALSE; // default is no OEM->ANSI conversion g_hwndList = NULL; // g_hwndParent = the caller, usually NSIS installer. if (g_hwndParent) // The window class name for dialog boxes is "#32770" - g_hwndList = FindWindowEx(FindWindowEx(g_hwndParent,NULL,_T("#32770"),NULL),NULL,_T("SysListView32"),NULL); + g_hwndList = FindWindowEx(FindWindowEx(g_hwndParent, NULL, _T("#32770"), NULL), NULL, _T("SysListView32"), NULL); // g_exec is the complete command to run: It has the copy of this DLL turned // into an executable right now. @@ -219,12 +270,17 @@ popstring(pExec); if (my_strstr(pExec, _T("/TIMEOUT=")) == pExec) { TCHAR *szTimeout = pExec + 9; - g_to = my_atoi(szTimeout); + to = my_atoi(szTimeout); *pExec = 0; goto params; } if (!lstrcmpi(pExec, _T("/OEM"))) { - bOEM = TRUE; + bOEM = forceNarrowInput = TRUE; + *pExec = 0; + goto params; + } + if (!lstrcmpi(pExec, _T("/MBCS"))) { + forceNarrowInput = TRUE; *pExec = 0; goto params; } @@ -242,155 +298,197 @@ // Got all the params off the stack. { - STARTUPINFO si={sizeof(si),}; - SECURITY_ATTRIBUTES sa={sizeof(sa),}; - SECURITY_DESCRIPTOR sd={0,}; - PROCESS_INFORMATION pi={0,}; + STARTUPINFO si = { sizeof(si), }; + SECURITY_ATTRIBUTES sa = { sizeof(sa), }; + SECURITY_DESCRIPTOR sd = { 0, }; + PROCESS_INFORMATION pi; const BOOL isNT = sizeof(void*) > 4 || (GetVersion() < 0x80000000); - HANDLE newstdout=0,read_stdout=0; - HANDLE newstdin=0,read_stdin=0; - DWORD dwRead = 1; - DWORD dwExit = 0; - DWORD dwWait = WAIT_TIMEOUT; - DWORD dwLastOutput; - static TCHAR szBuf[1024]; -#ifdef _UNICODE - static char ansiBuf[1024]; + HANDLE newstdout = 0, read_stdout = 0; + HANDLE newstdin = 0, read_stdin = 0; + int utfSource = sizeof(TCHAR) > 1 && !forceNarrowInput ? -1 : FALSE, utfOutput = sizeof(TCHAR) > 1; + DWORD cbRead, dwLastOutput; + DWORD dwExit = 0, waitResult = WAIT_TIMEOUT; + static BYTE bufSrc[1024]; + BYTE *pSrc; + SIZE_T cbSrcTot = sizeof(bufSrc), cbSrc = 0, cbSrcFree; + TCHAR *bufOutput = 0, *pNewAlloc, *pD; + SIZE_T cchAlloc, cbAlloc, cchFree; +#ifndef _MSC_VER // Avoid GCC "may be used uninitialized in this function" warnings + pD = NULL; + cchAlloc = 0; #endif - HGLOBAL hUnusedBuf = NULL; - TCHAR *szUnusedBuf = 0; - if (log) { - hUnusedBuf = GlobalAlloc(GHND, log & 2 ? (g_stringsize*sizeof(TCHAR)) : sizeof(szBuf)*4); // Note: will not grow if (log & 2) - if (!hUnusedBuf) { + pi.hProcess = pi.hThread = NULL; + codepage = bOEM ? CP_OEMCP : CP_ACP; + + if (!ignoreData) { + cbAlloc = stackMode ? (g_stringsize * sizeof(TCHAR)) : sizeof(bufSrc) * 4, cchAlloc = cbAlloc / sizeof(TCHAR); + pD = bufOutput = GlobalAlloc(GPTR, cbAlloc + sizeof(TCHAR)); // Include "hidden" space for a \0 + if (!bufOutput) { lstrcpy(szRet, _T("error")); goto done; } - szUnusedBuf = (TCHAR *)GlobalLock(hUnusedBuf); + *bufOutput = _T('\0'); } - sa.bInheritHandle = true; + sa.bInheritHandle = TRUE; sa.lpSecurityDescriptor = NULL; if (isNT) { - InitializeSecurityDescriptor(&sd,SECURITY_DESCRIPTOR_REVISION); - SetSecurityDescriptorDacl(&sd,true,NULL,false); + InitializeSecurityDescriptor(&sd, SECURITY_DESCRIPTOR_REVISION); + SetSecurityDescriptorDacl(&sd, TRUE, NULL, FALSE); sa.lpSecurityDescriptor = &sd; } - if (!CreatePipe(&read_stdout,&newstdout,&sa,0)) { + if (!CreatePipe(&read_stdout, &newstdout, &sa, 0)) { lstrcpy(szRet, _T("error")); goto done; } - if (!CreatePipe(&read_stdin,&newstdin,&sa,0)) { + if (!CreatePipe(&read_stdin, &newstdin, &sa, 0)) { lstrcpy(szRet, _T("error")); goto done; } - GetStartupInfo(&si); + GetStartupInfo(&si); // Why? si.dwFlags = STARTF_USESTDHANDLES|STARTF_USESHOWWINDOW; si.wShowWindow = SW_HIDE; si.hStdInput = newstdin; si.hStdOutput = newstdout; si.hStdError = newstdout; - if (!CreateProcess(NULL,g_exec,NULL,NULL,TRUE,CREATE_NEW_CONSOLE,NULL,NULL,&si,&pi)) { + if (!CreateProcess(NULL, g_exec, NULL, NULL, TRUE, CREATE_NEW_CONSOLE, NULL, NULL, &si, &pi)) { lstrcpy(szRet, _T("error")); goto done; } + // Now I'm talking with an executable copy of myself. dwLastOutput = GetTickCount(); + for (;;) { + TCHAR bufCh[2]; +waitForProcess: + waitResult = WaitForSingleObject(pi.hProcess, 0); + GetExitCodeProcess(pi.hProcess, &dwExit); +readMore: + PeekNamedPipe(read_stdout, 0, 0, 0, &cbRead, NULL); + if (!cbRead) { + if (waitResult == WAIT_OBJECT_0) { + break; // No data in the pipe and process ended, we are done + } + if (to && GetTickCount() > dwLastOutput+to) { + TerminateProcess(pi.hProcess, -1); + lstrcpy(szRet, _T("timeout")); + } + else { + Sleep(LOOPTIMEOUT); + } + continue; + } - // Now I'm talking with an executable copy of myself. - while (dwWait != WAIT_OBJECT_0 || dwRead) { - PeekNamedPipe(read_stdout, 0, 0, 0, &dwRead, NULL); - if (dwRead) { - dwLastOutput = GetTickCount(); -#ifdef _UNICODE - ReadFile(read_stdout, ansiBuf, sizeof(ansiBuf)-1, &dwRead, NULL); - ansiBuf[dwRead] = 0; - WideConvertIfASCII(ansiBuf, dwRead, szBuf, sizeof(szBuf)/sizeof(szBuf[0])); + dwLastOutput = GetTickCount(); + ReadFile(read_stdout, bufSrc + cbSrc, (DWORD) (cbSrcFree = cbSrcTot - cbSrc), &cbRead, NULL); + cbSrcFree -= cbRead, cbSrc = cbSrcTot - cbSrcFree; + pSrc = bufSrc; + + if (utfSource < 0 && cbSrc) { // Simple UTF-16LE detection +#ifdef UNICODE + utfSource = IsTextUnicode(pSrc, (UINT) (cbSrc & ~1), NULL) != FALSE; #else - ReadFile(read_stdout, szBuf, sizeof(szBuf)-1, &dwRead, NULL); - szBuf[dwRead] = '\0'; + utfSource = (cbSrc >= 3 && pSrc[0] && !pSrc[1]) || (cbSrc > 4 && pSrc[2] && !pSrc[3]); // Lame latin-only test + utfSource |= (cbSrc > 3 && pSrc[0] == 0xFF && pSrc[1] == 0xFE && (pSrc[2] | pSrc[3])); // Lame BOM test #endif - if (log) { - if (log & 2) { - lstrcpyn(szUnusedBuf + lstrlen(szUnusedBuf), szBuf, g_stringsize - lstrlen(szUnusedBuf)); + } + + if (ignoreData) { + cbSrc = 0; // Overwrite the whole buffer every read + continue; + } + + if (!cbRead) { + continue; // No new data, read more before trying to parse + } + +parseLines: + cchFree = cchAlloc - (pD - bufOutput); + for (;;) { + DWORD cbSrcChar = 1, cchDstChar, i; + *pD = _T('\0'); // Terminate output buffer because we can unexpectedly run out of data + if (!cbSrc) { + goto readMore; + } + + if (utfSource) { // UTF-16LE --> ?: + if (cbSrc < 2) { + goto readMore; + } + if (utfOutput) { // UTF-16LE --> UTF-16LE: + bufCh[0] = ((TCHAR*)pSrc)[0], cbSrcChar = sizeof(WCHAR), cchDstChar = 1; // We only care about certain ASCII characters so we don't bother dealing with surrogate pairs. + } + else { // UTF-16LE --> DBCS + // TODO: This is tricky because we need the complete base character (or surrogate pair) and all the trailing combining characters for a grapheme in the buffer before we can call WideCharToMultiByte. + utfOutput = FALSE; // For now we just treat it as DBCS + continue; } - else { - TCHAR *p, *p2; - SIZE_T iReqLen = lstrlen(szBuf) + lstrlen(szUnusedBuf) + 1; - if (GlobalSize(hUnusedBuf) < iReqLen*sizeof(TCHAR)) { - GlobalUnlock(hUnusedBuf); - hUnusedBuf = GlobalReAlloc(hUnusedBuf, iReqLen*sizeof(TCHAR)+sizeof(szBuf), GHND); - if (!hUnusedBuf) { - lstrcpy(szRet, _T("error")); - break; - } - szUnusedBuf = (TCHAR *)GlobalLock(hUnusedBuf); + } + else { // DBCS --> ?: + if (utfOutput) { // DBCS --> UTF-16LE: + BOOL isMb = IsDBCSLeadByteEx(codepage, ((CHAR*)pSrc)[0]); + if (isMb && cbSrc < ++cbSrcChar) { + goto readMore; } - p = szUnusedBuf; // get the old left overs - lstrcat(p, szBuf); - while ((p = my_strstr(p, _T("\t")))) { - if ((int)(p - szUnusedBuf) > (int)(GlobalSize(hUnusedBuf)/sizeof(TCHAR) - TAB_REPLACE_SIZE - 1)) - { - *p++ = _T(' '); - } + cchDstChar = MultiByteToWideChar(codepage, 0, (CHAR*)pSrc, cbSrcChar, (WCHAR*) bufCh, 2); + } + else { // DBCS --> DBCS: + bufCh[0] = ((CHAR*)pSrc)[0], cchDstChar = 1; // Note: OEM codepage will be converted by LogMessage + } + } + + if (bufCh[0] == _T('\t') && tabExpandLength) { // Expand tab to spaces? + if (cchFree < tabExpandLength) { + goto resizeOutputBuffer; + } + lstrcpy(pD, TAB_REPLACE); + pD += tabExpandLength, cchFree -= tabExpandLength; + } + else if (bufCh[0] == _T('\r') && logMode) { + // Eating it + } + else if (bufCh[0] == _T('\n') && logMode) { + LogMessage(bufOutput, bOEM); // Output has already been \0 terminated + *(pD = bufOutput) = _T('\0'), cchFree = cchAlloc; + } + else { + if (cchFree < cchDstChar) { + SIZE_T cchOrgOffset; +resizeOutputBuffer: + if (stackMode) { + ignoreData = TRUE; // Buffer was already maximum for the NSIS stack, we cannot handle more data + if (utfOutput) + TruncateStringUTF16LE((LPWSTR) bufOutput, pD - bufOutput, (LPCWSTR) bufCh, cchDstChar); else - { - int len = lstrlen(p); - TCHAR *c_out=(TCHAR*)p+TAB_REPLACE_SIZE+len; - TCHAR *c_in=(TCHAR *)p+len; - while (len-- > 0) { - *c_out--=*c_in--; - } - - lstrcpy(p, TAB_REPLACE); - p += TAB_REPLACE_SIZE; - *p = _T(' '); - } - } - - p = szUnusedBuf; // get the old left overs - for (p2 = p; *p2;) { - if (*p2 == _T('\r')) { - *p2++ = 0; - continue; - } - if (*p2 == _T('\n')) { - *p2 = 0; - while (!*p && p != p2) p++; - LogMessage(p, bOEM); - p = ++p2; - continue; - } - p2 = CharNext(p2); + TruncateStringMB(codepage, (LPSTR) bufOutput, pD - bufOutput, bufCh[0]); + goto waitForProcess; } - - // If data was taken out from the unused buffer, move p contents to the start of szUnusedBuf - if (p != szUnusedBuf) { - TCHAR *p2 = szUnusedBuf; - while (*p) *p2++ = *p++; - *p2 = 0; + cchAlloc += 1024, cbAlloc = cchAlloc / sizeof(TCHAR); + pNewAlloc = GlobalReAlloc(bufOutput, cbAlloc + sizeof(TCHAR),GPTR|GMEM_MOVEABLE); // Include "hidden" space for a \0 + if (!pNewAlloc) { + lstrcpy(szRet, _T("error")); + ignoreData = TRUE; + goto waitForProcess; } + cchOrgOffset = pD - bufOutput; + *(pD = (bufOutput = pNewAlloc) + cchOrgOffset) = _T('\0'); + goto parseLines; + } + for (i = 0; i < cchDstChar; ++i) { + *pD++ = bufCh[i], --cchFree; } } + pSrc += cbSrcChar, cbSrc -= cbSrcChar; } - else { - if (g_to && GetTickCount() > dwLastOutput+g_to) { - TerminateProcess(pi.hProcess, -1); - lstrcpy(szRet, _T("timeout")); - } - else Sleep(LOOPTIMEOUT); - } - - dwWait = WaitForSingleObject(pi.hProcess, 0); - GetExitCodeProcess(pi.hProcess, &dwExit); - PeekNamedPipe(read_stdout, 0, 0, 0, &dwRead, NULL); } + done: - if (log & 2) pushstring(szUnusedBuf); - if (log & 1 && *szUnusedBuf) LogMessage(szUnusedBuf, bOEM); - if ( dwExit == STATUS_ILLEGAL_INSTRUCTION ) + if (stackMode) pushstring(bufOutput); + if (logMode && *bufOutput) LogMessage(bufOutput,bOEM); // Write remaining output + if (dwExit == STATUS_ILLEGAL_INSTRUCTION) lstrcpy(szRet, _T("error")); if (!szRet[0]) wsprintf(szRet,_T("%d"),dwExit); pushstring(szRet); @@ -402,33 +500,14 @@ CloseHandle(read_stdin); if (pExec-2 >= g_exec) *(pExec-2) = _T('\0'); // skip space and quote - if (executor) DeleteFile(executor); + if (executor) + DeleteFile(executor); GlobalFree(g_exec); - if (log) { - GlobalUnlock(hUnusedBuf); - GlobalFree(hUnusedBuf); - } + if (bufOutput) + GlobalFree(bufOutput); } } -// Tim Kosse's LogMessage -void LogMessage(const TCHAR *pStr, BOOL bOEM) { - LVITEM item={0}; - int nItemCount; - if (!g_hwndList) return; - //if (!lstrlen(pStr)) return; -#ifndef _UNICODE - if (bOEM == TRUE) OemToCharBuff(pStr, (char*)pStr, lstrlen(pStr)); -#endif - nItemCount=(int) SendMessage(g_hwndList, LVM_GETITEMCOUNT, 0, 0); - item.mask=LVIF_TEXT; - item.pszText=(TCHAR *)pStr; - item.cchTextMax=0; - item.iItem=nItemCount; - ListView_InsertItem(g_hwndList, &item); - ListView_EnsureVisible(g_hwndList, item.iItem, 0); -} - TCHAR *my_strstr(TCHAR *a, TCHAR *b) { int l = lstrlen(b); ```
jaimergp commented 2 years ago

Good news, I could reproduce the error, found the cause and provided a fix in #563!

If you need it fixed urgently, the patch can be as simple as changing this line:

https://github.com/conda/constructor/blob/de1ce16c62ff4d748e2d4a8046e923d3136e14db/constructor/nsis/main.nsi.tmpl#L859

To

-        nsExec::ExecToLog $2
+        nsExec::Exec $2

This will reduce the verbosity of the Details panel, though. The PR introduces a method to minimize this side effect as much as possible.

scottwsides commented 2 years ago

Thanks!!!

JinYongWu commented 1 year ago

What is the earliest version of Constructor which has this fix?

jaimergp commented 1 year ago

3.4.0, but realistically speaking, 3.4.1 (hotfix for 3.4.0).