From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: MPS: Win64 testers? Date: Wed, 07 Aug 2024 14:41:15 +0300 Message-ID: <86ed70o90k.fsf@gnu.org> References: <86h6bxq3p3.fsf@gnu.org> <86sevho61y.fsf@gnu.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="33679"; mail-complaints-to="usenet@ciao.gmane.io" Cc: emacs-devel@gnu.org, pipcet@protonmail.com To: Quang Kien Nguyen Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Wed Aug 07 13:43:04 2024 Return-path: Envelope-to: ged-emacs-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1sbf4J-0008Xi-2J for ged-emacs-devel@m.gmane-mx.org; Wed, 07 Aug 2024 13:43:03 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sbf2j-0006oL-UK; Wed, 07 Aug 2024 07:41:25 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sbf2g-0006cq-Jb for emacs-devel@gnu.org; Wed, 07 Aug 2024 07:41:23 -0400 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1sbf2e-0003Dm-7W; Wed, 07 Aug 2024 07:41:20 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=5tI+rUalzNJYlyEfP+Vt9JgVFgpAbryely4729n2VBM=; b=LPOLeWERANbr NVl0/7uefhBd8BSNfvUsam2jI//ySK/3YEALbbqKFiGFy2Xh6hg2rRWsfWEPztVxpYI7SnKlB/zPU vHuauNA7rdxE8xwNIbnU/jESZygWsQvW4IXPNIjHCWjb0l02SG44Zg7WD822DLVNRBY+93SlDYOVO GHF83LCVFJyy0u47TDpsL1k/2h+/flgiuS/JADGTP0i9BXGKJkxOQG/Lc2m0fLsHYmU+tNVtGMeGv l1rk1IHAFS7VGmU5KHB96By2ZsYUY8C+cKtgxtXXGdobmGG2/FlUj60YB2W4K4NWnI4Gyur0ZmsjG p0wL+ID8jUU45amJ8wHRPQ==; In-Reply-To: (message from Quang Kien Nguyen on Tue, 6 Aug 2024 13:52:27 -0700) X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.devel:322497 Archived-At: > From: Quang Kien Nguyen > Date: Tue, 6 Aug 2024 13:52:27 -0700 > Cc: emacs-devel@gnu.org, pipcet@protonmail.com > > > I'm not sure I understand what you are asking. Are you asking how to > > test that the standard handles aren't inherited by subprocesses? > > I can check that the SetHandleInformation is returning TRUE so the handles > shouldn't be inherited already. However, is there any Emacs feature > that will be > broken if the handles are inherited? I would like to test that to make sure the > E2E flow is still working. This is not about some Emacs feature, or at least not only about that. This is about having Emacs on Windows behave the same as Emacs on Posix systems, wrt inheriting handles to child sub-processes. We want this to be done so that anyone who writes a Lisp program that launches sub-processes could be certain they will behave the same wrt inheritance of standard handles, on all supported systems. To test that, one would need to demonstrate that the parent Emacs process can do something with its standard handles without affecting the same handles of the child process. For example, moving the file pointer in one process doesn't move it in the other. As another example, if Emacs's standard output is redirected to a file, and writing to stdout in the child process writes to that same file, then the standard output handle _was_ inherited. Yet another possibility is to use a tool such as ProcessExplorer to show the target of each handle, in both parent Emacs and its child sub-process, and verify the target is different. > > Btw, I still don't think I've seen the patch you are want to test, so > > I don't even know if it's clean and correct by itself. > > The patch is here > https://raw.githubusercontent.com/kiennq/emacs-build/main/patches/0004-init_ntproc-Use-SetHandleInformation-to-set-NOINHERI.patch > It's also attached in the mail. Thanks, see some comments below. > --- a/src/w32.c > +++ b/src/w32.c > @@ -7783,10 +7783,6 @@ init_winsock (int load_now) > if (winsock_lib != NULL) > return TRUE; > > - pfn_SetHandleInformation > - = (void *) get_proc_addr (GetModuleHandle ("kernel32.dll"), > - "SetHandleInformation"); > - > winsock_lib = LoadLibrary ("Ws2_32.dll"); > > if (winsock_lib != NULL) > @@ -10471,6 +10467,10 @@ init_ntproc (int dumping) > Conveniently, init_environment is called before us, so > PRELOAD_WINSOCK can be set in the registry. */ > > + pfn_SetHandleInformation > + = (void *) get_proc_addr (GetModuleHandle ("kernel32.dll"), > + "SetHandleInformation"); > + This hunk is not needed, for the reasons explained below. > @@ -10480,60 +10480,73 @@ init_ntproc (int dumping) > /* Initial preparation for subprocess support: replace our standard > handles with non-inheritable versions. */ > { > - HANDLE parent; > - HANDLE stdin_save = INVALID_HANDLE_VALUE; > - HANDLE stdout_save = INVALID_HANDLE_VALUE; > - HANDLE stderr_save = INVALID_HANDLE_VALUE; > - > - parent = GetCurrentProcess (); > - > - /* ignore errors when duplicating and closing; typically the > - handles will be invalid when running as a gui program. */ > - DuplicateHandle (parent, > - GetStdHandle (STD_INPUT_HANDLE), > - parent, > - &stdin_save, > - 0, > - FALSE, > - DUPLICATE_SAME_ACCESS); > - > - DuplicateHandle (parent, > - GetStdHandle (STD_OUTPUT_HANDLE), > - parent, > - &stdout_save, > - 0, > - FALSE, > - DUPLICATE_SAME_ACCESS); > - > - DuplicateHandle (parent, > - GetStdHandle (STD_ERROR_HANDLE), > - parent, > - &stderr_save, > - 0, > - FALSE, > - DUPLICATE_SAME_ACCESS); > - > - fclose (stdin); > - fclose (stdout); > - fclose (stderr); > - > - if (stdin_save != INVALID_HANDLE_VALUE) > - _open_osfhandle ((intptr_t) stdin_save, O_TEXT); > + /* For UCRT, the _fdopen will try to find free stream from > + _IOB_ENTRIES (= 3), thus we can't reopen the standard handles > + with it. Using SetHandleInformation to make the handle not > + inheritable to child process is a better way. */ This is not what I think we need. First, there's no need to change anything in the MSVCRT build, because AFAIK the current code "just works" in such a build, both in 32-bit builds and in 64-bit builds. The problem happens only in UCRT builds. Second, UCRT builds are only supported on versions of Windows where SetHandleInformation is always available; on older Windows systems, the UCRT build will refuse to run, because UCRT is not available. So what I think is needed is a compile-time condition that makes the UCRT build use SetHandleInformation instead of the DuplicateHandle/fclose/_fdopen dance, and leaves the existing code intact in non-UCRT builds. I think "#ifdef _UCRT" is such a condition, but I'm not 100% sure.