From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Alan Third Newsgroups: gmane.emacs.bugs Subject: bug#75275: 30.0.92; `make-thread` bug on macOS 15.2 Date: Thu, 2 Jan 2025 11:03:50 +0000 Message-ID: References: <86bjwplmc1.fsf@gnu.org> <865xmxlivo.fsf@gnu.org> <86y0ztk323.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="4105"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 75275@debbugs.gnu.org, Eli Zaretskii , stefankangas@gmail.com To: Gerd =?UTF-8?Q?M=C3=B6llmann?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Thu Jan 02 12:05:26 2025 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1tTJ14-0000wZ-9k for geb-bug-gnu-emacs@m.gmane-mx.org; Thu, 02 Jan 2025 12:05:26 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1tTJ0j-000164-Gd; Thu, 02 Jan 2025 06:05:05 -0500 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 1tTJ0i-00014i-45 for bug-gnu-emacs@gnu.org; Thu, 02 Jan 2025 06:05:04 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1tTJ0h-0001I8-PW for bug-gnu-emacs@gnu.org; Thu, 02 Jan 2025 06:05:03 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=In-Reply-To:MIME-Version:References:From:Date:To:Subject; bh=VIrjXXX/K7w5zWouMHfb8Ie/SSG8u08fP+ci1J+0IFA=; b=nLOq9gNfw2GA4CY+YEUD04iImJFlbYoD9gf/N2d/7fSuin9GQxI4OpLGmVtWdKOOgX6f1Lwn9B0DubZbQTm6GUDaTTyZMdsFq7kevLXWmA8CaH8TLVpt/NG5qqbETjQK4iw9ek3GWjiqDO+KFx6yuKIscQQ8EJV2nDL0L5NchkTAHu+1Vgsr45HzMswzgqhwEnM64GHYEoPhddkP/34qprMYbops8RkASKy0LAdc7RkZ2CVKooESp45+QcbBm0NAHt8O3+lJ9l0nAI7uzprWdgm9HsE5MDjiCX2Ieop4tRHhpfgAfJNF6o71fa+L2UqUa/WXWwMUkgOe8L/yMz/vWQ==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1tTJ0g-0002DP-K4 for bug-gnu-emacs@gnu.org; Thu, 02 Jan 2025 06:05:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Alan Third Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 02 Jan 2025 11:05:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 75275 X-GNU-PR-Package: emacs Original-Received: via spool by 75275-submit@debbugs.gnu.org id=B75275.17358158448444 (code B ref 75275); Thu, 02 Jan 2025 11:05:02 +0000 Original-Received: (at 75275) by debbugs.gnu.org; 2 Jan 2025 11:04:04 +0000 Original-Received: from localhost ([127.0.0.1]:42882 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1tTIzj-0002C7-T5 for submit@debbugs.gnu.org; Thu, 02 Jan 2025 06:04:04 -0500 Original-Received: from dane.soverin.net ([185.233.34.31]:57603) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.84_2) (envelope-from ) id 1tTIzg-0002Ba-64 for 75275@debbugs.gnu.org; Thu, 02 Jan 2025 06:04:02 -0500 Original-Received: from smtp.soverin.net (c04smtp-lb01.int.sover.in [10.10.4.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dane.soverin.net (Postfix) with ESMTPS id 4YP3jj4HNkz2xKd; Thu, 2 Jan 2025 11:03:53 +0000 (UTC) Original-Received: from smtp.soverin.net (smtp.soverin.net [10.10.4.100]) by soverin.net (Postfix) with ESMTPSA id 4YP3jh1w5yzF1; Thu, 2 Jan 2025 11:03:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=idiocy.org; s=soverin; t=1735815833; bh=IfS/terr+nzlvnsEE3GgW4Lch+1Eh9JScLA5Cv/KL1E=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=earxgMYg0LFXg2H6OWOIp8zc2zjftcwWihob65VuDSiGEkfulooHS7AgYCJlvl20x JrH7CDlnezchRKdcuzROSVNcUKzb89D9/WH1R7TWovgrpY18wxv/iRrIhYAJfqOrCP FWeupOfi2g957Ue+bOekAhl+BgIcON0aHe0JBAosV5aVEFjD1sySj/w2/lIAiXgg+M 1zl0VfQa0PB5ogEFpMeCW7k7HI0RFqShQnIW3uTLxP7CXjDLDbOrx0o3QmwVBohd+6 vmvzi8vos8wRmVazyLDlZ8G9GjpGJalSPzSErWbjlfp+qd23WjZw80w0Z/F3LLQst/ oxoq/LynNFG+w== X-CM-Analysis: v=2.4 cv=e8f8Sbp/ c=1 sm=1 tr=0 ts=67767299 a=UbsBXRcqaZ6D9kgPt/Dvnw==:617 a=xqWC_Br6kY4A:10 a=8nJEP1OIZ-IA:10 a=VdSt8ZQiCzkA:10 a=mDV3o1hIAAAA:8 a=pGLkceISAAAA:8 a=hIj89exaAAAA:8 a=FD9y6Vhym89-YHjGetcA:9 a=3ZKOabzyN94A:10 a=wPNLvfGTeEIA:10 a=lS9wXHQM5UdnNJ4u63Ry:22 a=9MSFP0l5Dcwi9NrB_JPx:22 X-CM-Envelope: MS4xfOGPni5aqlGCakHfbwq0r1K83bUk04D3gE/4v31FqzZmYVqXnJ/lAPmKqhOtCb1Kj75bt4RI9xNvDaDQnwOJ6BV7ismOXLQquLuMiMOMY4/Z5tpAaRPr yBBGO5F7n5uhlly3yzsWSzH9xMRgo+TzcprFKGWeVxbEdJKts1ryHQ0EAeK8g/OAn7Bq4LpOHNoRItJfh4+zQ2tVnqTMN4o3WCypLkeAmMP9eXHy6INIWG9X Ipwzwp7T1KygNQY4xhG57xnfRjaqf8cZMxWFdaKB+1TIqtGAgw7CuK5j2jSD7IIw Original-Received: from localhost (faroe.holly.idiocy.org [local]) by faroe.holly.idiocy.org (OpenSMTPD) with ESMTPA id 61f4fff6; Thu, 2 Jan 2025 11:03:50 +0000 (UTC) Mail-Followup-To: Alan Third , Gerd =?UTF-8?Q?M=C3=B6llmann?= , Eli Zaretskii , stefankangas@gmail.com, 75275@debbugs.gnu.org Content-Disposition: inline In-Reply-To: X-Spampanel-Class: ham X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:298158 Archived-At: On Thu, Jan 02, 2025 at 11:04:11AM +0100, Gerd Möllmann wrote: > Eli Zaretskii writes: > > >> From: Gerd Möllmann > >> Cc: stefankangas@gmail.com, alan@idiocy.org, 75275@debbugs.gnu.org > >> Date: Thu, 02 Jan 2025 09:41:38 +0100 > >> > >> Gerd Möllmann writes: > >> > >> > Eli Zaretskii writes: > >> > > >> >> So should we add a condition before calling [NSApp run] that we are in > >> >> the main thread? > >> > > >> > ATM, I don't understand how we land in that line in ns_select_1 if not > >> > [NSThread isMainThread]. Maybe I need new glasses. I asked Stefan if he > >> > can see something in LLDB. > >> > >> It must something in here: > >> > >> if (![NSThread isMainThread] > >> || (timeout && timeout->tv_sec == 0 && timeout->tv_nsec == 0)) > >> thread_select (pselect, nfds, readfds, writefds, > >> exceptfds, timeout, sigmask); > >> > >> Should we return here? Yes. We used to be but it was removed: 9370a4763aacbb9278b5be9c92a2484e3652bc29 I don't know why it was removed, but I'd bet that, at the very least, the isMainThread check should have moved with the 'NSApp == nil' check. > > I don't know. Is there anything in the following code that can be > > relevant to a non-main thread? Note that non-main threads can > > legitimately call wait_reading_process_output, which calls ns_select. > > For example, what happens if a non-main Lisp thread starts a > > sub-process? we do expect to be able to read the output from that > > sub-process. My take on how this works was that in a non-main thread ns_select should just act like pselect, hence it used to literally just call pselect and return. This would hopefully allow Emacs to handle IO correctly without having to make the NS runloop code do things it can't do. FWIW, I still think the NS code in its current form is unsuitable for multi-threaded use and must be rewritten. > Really hard to tell. Perhaps someone could try to follow what I write > below and tell if it makes sense? Everything in ns_select_1. > > 1. I think this code must run in a non-main thread: > > if (nr > 0) > { > pthread_mutex_lock (&select_mutex); > ... set some variables ... > /* Inform fd_handler that select should be called. */ > c = 'g'; > emacs_write_sig (selfds[1], &c, 1); > } > > selfds is apparently some pipe, NS-specific. The function fd_handler is > called when writing to the pipe I assume. fd_handler is set up like > this > > [NSThread detachNewThreadSelector:@selector (fd_handler:) > toTarget:NSApp > withObject:nil]; > > Looks to me like it runs in a thread of its own. fd_handler then > pselects on the fd sets set in the if above. That looks like it is > relevant to reading process output. And that means we may _not_ return > from ns_select_1 early when ![NSThread isMainThread]. > > else if (nr == 0 && timeout) > { > /* No file descriptor, just a timeout, no need to wake fd_handler. */ > double time = timespectod (*timeout); > timed_entry = [[NSTimer scheduledTimerWithTimeInterval: time > target: NSApp > selector: > @selector (timeout_handler:) > userInfo: 0 > repeats: NO] > retain]; > } No, none of that needs to run when we're not in the main thread. fd_handler run pselect in a separate thread because the NS main thread has to run the ns main thread run loop to handle incoming IO from the window system. The NS run loop can emulate parts of pselect, but not the whole thing, so we are required to run both the NS runloop and pselect simultaneously, hence fd_handler. If we don't need to run the runloop, i.e. we're in a non-main thread, then we can just run pselect directly and ignore fd_handler. > 2. This code > > else if (nr == 0 && timeout) > { > /* No file descriptor, just a timeout, no need to wake fd_handler. */ > double time = timespectod (*timeout); > timed_entry = [[NSTimer scheduledTimerWithTimeInterval: time > target: NSApp > selector: > @selector (timeout_handler:) > userInfo: 0 > repeats: NO] > retain]; > } > > means basically only to send an app-defined event after a timeout. I > interpret this as "leave the NS event loop to let Emacs do things > after a timeout". Looks okay to me. Correct. In more detail it sends an "App defined" event to the main thread which signals to the run loop to stop itself. > 3. This > > else /* No timeout and no file descriptors, can this happen? */ > { > /* Send appdefined so we exit from the loop. */ > ns_send_appdefined (-1); > } > > is likely also okay because send_app_defined has code checking for > being in the main thread. This will send the app defined event to the main thread run loop. The code in ns_send_appdefined actually instructs the main thread runloop to send itself the event if called from a non-main thread. > 4. The [NSApp run] follows, and it can under no circumstances be done > in a mon-main thread. We should put that in an if for sure. > > if ([NSThread isMainThread]) [NSApp run]; In this circumstance no. In *Step each thread has its own run loop and event queue, so if you call [NSApp run] on a sub-thread it will look at its own queue, which in this case likely has nothing on it, and it will run forever. That's by design, that's how you're supposed to write NS apps. We obviously don't want to do that. > 5. The code below is another enigma. > > I can't figure out why that is done, and what > last_appdefined_event_data is for. But since it is run today, I'd > propose to just let it run. I don't see that it does immediate harm. It's essentially just working out how the run loop was terminated (by fd_handler, by some window system input, or by timeout) and creating suitable return values by, for example, gathering the results of the pselect run in fd_handler. Basically, none of this needs to run in a sub-thread. We should be able to just run pselect directly and return. Perhaps there's some other edge case that prevents that, but I suspect it was just overlooked. After all, nobody understands this code. -- Alan Third