Re: [PATCH v2] Cygwin: pty: Revise code waiting for forwarding again.
Hi Corinna, On Fri, 24 Jan 2020 12:07:30 +0100 Corinna Vinschen wrote: > Too bad. It's pretty strange that CreatePseudoConsole returns a > valid HPCON but then isn't ready to take input immediately. > > > I do not come up with other implementation so far. > > > > Let me consider a while. > > I wonder how others solve this problem. I see that the native OpenSSH > is using Sleeps, too, in their start_with_pty() function, calling > AttachConsole in a loop, but I'm not sure if these are related to pseudo > console usage. The commit message don't explain anything there :( The essence of the difficulty is that we have to support both cygwin programs and native console apps. If we consider only of native console apps, any time we can use pseudo console. However, pseudo console is not transparent at all, so it cannot be used for cygwin programs. Therefore, current cygwin is switching handles to be used between named-pipe and pseudo console. However, because pseudo console has relatively long latency, if pipe is switched just after writing to pseudo console, the forwarding does not get in time. So the "wait" is needed before switching. I had tried WriteFile(), ReadFile() and DeviceIoControl() for HANDLE hConDrvReference, however, all atempts of them failed. -- Takashi Yano
Re: [PATCH] Cygwin: pty: Add missing console API hooks.
Hi Corinna, On Fri, 24 Jan 2020 11:26:27 +0100 Corinna Vinschen wrote: > On Jan 23 22:05, Takashi Yano wrote: > > On Thu, 23 Jan 2020 13:48:13 +0100 > > Corinna Vinschen wrote: > > > On Jan 23 13:33, Takashi Yano wrote: > > > > - Following console APIs are additionally hooked for cygwin programs > > > > which directly call them. > > > > * FillConsoleOutputAttribute() > > > > * FillConsoleOutputCharacterA() > > > > * FillConsoleOutputCharacterW() > > > > * ScrollConsoleScreenBufferA() > > > > * ScrollConsoleScreenBufferW() > > > > > > Which Cygwin programs are doing that? They wouldn't work correctly in > > > ptys anyway, isn't it? Does it really make sense to make them happy > > > rather than requesting to change them? > > > > Just a possibility. There is no specific example. > > In that case I'd prefer not to apply this patch. Using native Windows > console functions in a Cygwin application just doesn't make sense, and > we shouldn't support that beyond what's necessary for older, existing > applications. I searched in /bin and found that many of cygwin programs calls win32 api directly. However, the only cygwin programs which calls console APIs are cygrunsrv.exe, gdb.exe and w3m.exe in my installation. cygrunsrv calls: DLL Name: ADVAPI32.dll vma: Hint/Ord Member-Name Bound-To 264ec 87 CloseServiceHandle 26502 92 ControlService 26514 128 CreateServiceA 26526 218 DeleteService 26536 255 EnumServicesStatusA 2654c 511 OpenSCManagerA 2655e 513 OpenServiceA 2656e 554 QueryServiceConfigA 26584 559 QueryServiceStatus 2659a 568 RegCloseKey 265a8 569 RegConnectRegistryA 265be 576 RegCreateKeyExA 265d0 601 RegEnumValueA 265e0 603 RegFlushKey 265ee 616 RegOpenKeyExA 265fe 629 RegQueryValueExA 26612 645 RegSetValueExA 26624 654 RegisterServiceCtrlHandlerExA 26644 712 SetServiceStatus 26658 718 StartServiceA 26668 719 StartServiceCtrlDispatcherA DLL Name: KERNEL32.dll vma: Hint/Ord Member-Name Bound-To 26686 16 AllocConsole <== 26696 83 CloseHandle 266a4 239 EnterCriticalSection 266bc 351 FormatMessageA 266ce 353 FreeConsole <== 266dc 356 FreeLibrary 266ea 482 GetExitCodeProcess 26700 515 GetLastError 26710 531 GetModuleFileNameA 26726 533 GetModuleHandleA 2673a 581 GetProcAddress 2674c 663 GetTickCount 2675c 677 GetVersion 2676a 747 InitializeCriticalSection 26786 806 LeaveCriticalSection 2679e 809 LoadLibraryA 267ae 879 OpenProcess 267bc1035 SetConsoleTitleA <== 267d01078 SetLastError 267e01140 Sleep 267e81218 WaitForSingleObject DLL Name: USER32.dll vma: Hint/Ord Member-Name Bound-To 267fe 13 BringWindowToTop 26812 260 GetForegroundWindow 26828 335 GetTopWindow 26838 538 SetForegroundWindow gdb calls: DLL Name: KERNEL32.dll vma: Hint/Ord Member-Name Bound-To 60a5bc 84 CloseHandle 60a5ca106 ContinueDebugEvent 60a5e0140 CreateFileA 60a5ee172 CreateProcessW 60a600201 DebugActiveProcess 60a616348 FlushInstructionCache 60a62e358 FreeLibrary 60a63c364 GenerateConsoleCtrlEvent <== 60a658440 GetConsoleScreenBufferInfo <== 60a676454 GetCurrentProcess 60a68a485 GetExitCodeThread 60a69e517 GetLastError 60a6ae535 GetModuleHandleA 60a6c2538 GetModuleHandleW 60a6d6583 GetProcAddress 60a6e8628 GetSystemDirectoryW 60a6fe651 GetThreadContext 60a712661 GetThreadSelectorEntry 60a72c811 LoadLibraryA 60a73c882 OpenProcess 60a74a950 ReadProcessMemory 60a75e988 ResumeThread 60a76e 1015 SetConsoleCtrlHandler <== 60a786 1057 SetEnvironmentVariableW 60a7a0 1059 SetEvent 60a7ac 1115 SetThreadContext 60a7c0 1154 SuspendThread 60a7d0 1160 TerminateProcess 60a7e4 1221 WaitForDebugEvent 60a7f8 1224 WaitForSingleObject 60a80e 1278 WriteProcessMemory w3m calls: DLL Name: KERNEL32.dll vma: Hint/Ord Member-Name Bound-To 1384c0 83 CloseHandle 1384ce139 CreateFileA 1384dc356 FreeLibrary 1384ea441 GetConsoleTitle
[PATCH] Cygwin: console: Share readahead buffer within the same process.
- The cause of the problem reported in
https://www.cygwin.com/ml/cygwin/2020-01/msg00220.html is that the
chars input before dup() cannot be read from the new file descriptor.
This is because the readahead buffer (rabuf) in the console is newly
created by dup(), and does not inherit from the parent. This patch
fixes the issue.
---
winsup/cygwin/fhandler.cc | 58 ---
winsup/cygwin/fhandler.h | 24 -
winsup/cygwin/fhandler_console.cc | 16 -
winsup/cygwin/fhandler_termios.cc | 35 ++-
winsup/cygwin/fhandler_tty.cc | 2 +-
5 files changed, 80 insertions(+), 55 deletions(-)
diff --git a/winsup/cygwin/fhandler.cc b/winsup/cygwin/fhandler.cc
index aeee8fe4d..ad4a7e61c 100644
--- a/winsup/cygwin/fhandler.cc
+++ b/winsup/cygwin/fhandler.cc
@@ -44,11 +44,12 @@ void
fhandler_base::reset (const fhandler_base *from)
{
pc << from->pc;
- rabuf = NULL;
- ralen = 0;
- raixget = 0;
- raixput = 0;
- rabuflen = 0;
+ ra.rabuf = NULL;
+ ra.ralen = 0;
+ ra.raixget = 0;
+ ra.raixput = 0;
+ ra.rabuflen = 0;
+ set_rabuf ();
_refcnt = 0;
}
@@ -66,15 +67,15 @@ int
fhandler_base::put_readahead (char value)
{
char *newrabuf;
- if (raixput < rabuflen)
+ if (raptr->raixput < raptr->rabuflen)
/* Nothing to do */;
- else if ((newrabuf = (char *) realloc (rabuf, rabuflen += 32)))
-rabuf = newrabuf;
+ else if ((newrabuf = (char *) realloc (raptr->rabuf, raptr->rabuflen += 32)))
+raptr->rabuf = newrabuf;
else
return 0;
- rabuf[raixput++] = value;
- ralen++;
+ raptr->rabuf[raptr->raixput++] = value;
+ raptr->ralen++;
return 1;
}
@@ -82,11 +83,11 @@ int
fhandler_base::get_readahead ()
{
int chret = -1;
- if (raixget < ralen)
-chret = ((unsigned char) rabuf[raixget++]) & 0xff;
+ if (raptr->raixget < raptr->ralen)
+chret = ((unsigned char) raptr->rabuf[raptr->raixget++]) & 0xff;
/* FIXME - not thread safe */
- if (raixget >= ralen)
-raixget = raixput = ralen = 0;
+ if (raptr->raixget >= raptr->ralen)
+raptr->raixget = raptr->raixput = raptr->ralen = 0;
return chret;
}
@@ -94,10 +95,10 @@ int
fhandler_base::peek_readahead (int queryput)
{
int chret = -1;
- if (!queryput && raixget < ralen)
-chret = ((unsigned char) rabuf[raixget]) & 0xff;
- else if (queryput && raixput > 0)
-chret = ((unsigned char) rabuf[raixput - 1]) & 0xff;
+ if (!queryput && raptr->raixget < raptr->ralen)
+chret = ((unsigned char) raptr->rabuf[raptr->raixget]) & 0xff;
+ else if (queryput && raptr->raixput > 0)
+chret = ((unsigned char) raptr->rabuf[raptr->raixput - 1]) & 0xff;
return chret;
}
@@ -105,7 +106,7 @@ void
fhandler_base::set_readahead_valid (int val, int ch)
{
if (!val)
-ralen = raixget = raixput = 0;
+raptr->ralen = raptr->raixget = raptr->raixput = 0;
if (ch != -1)
put_readahead (ch);
}
@@ -1092,7 +1093,7 @@ fhandler_base::lseek (off_t offset, int whence)
if (whence != SEEK_CUR || offset != 0)
{
if (whence == SEEK_CUR)
- offset -= ralen - raixget;
+ offset -= raptr->ralen - raptr->raixget;
set_readahead_valid (0);
}
@@ -1142,7 +1143,7 @@ fhandler_base::lseek (off_t offset, int whence)
readahead that we have to take into account when calculating
the actual position for the application. */
if (whence == SEEK_CUR)
-res -= ralen - raixget;
+res -= raptr->ralen - raptr->raixget;
return res;
}
@@ -1565,23 +1566,24 @@ fhandler_base::fhandler_base () :
ino (0),
_refcnt (0),
openflags (0),
- rabuf (NULL),
- ralen (0),
- raixget (0),
- raixput (0),
- rabuflen (0),
unique_id (0),
archetype (NULL),
usecount (0)
{
+ ra.rabuf = NULL;
+ ra.ralen = 0;
+ ra.raixget = 0;
+ ra.raixput = 0;
+ ra.rabuflen = 0;
+ set_rabuf ();
isclosed (false);
}
/* Normal I/O destructor */
fhandler_base::~fhandler_base ()
{
- if (rabuf)
-free (rabuf);
+ if (ra.rabuf)
+free (ra.rabuf);
}
/**/
diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index c0d56b4da..35190c0fe 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -202,16 +202,22 @@ class fhandler_base
ino_t ino; /* file ID or hashed filename, depends on FS. */
LONG _refcnt;
+ public:
+ struct rabuf_t
+ {
+char *rabuf; /* used for crlf conversion in text files */
+size_t ralen;
+size_t raixget;
+size_t raixput;
+size_t rabuflen;
+ };
protected:
/* File open flags from open () and fcntl () calls */
int openflags;
- char *rabuf; /* used for crlf conversion in text files */
- size_t ralen;
- size_t raixget;
- size_t raixput;
- size_t rabuflen;
+ struct rabuf_t ra;
+ struct rabuf_t *raptr;
/* Used for advisory file locking. See flock.cc. */
int64_t unique_id;
@@ -315,7 +321,8 @@ class fhandler_base
