From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Support threads in modules Date: Wed, 26 Apr 2017 08:23:27 +0300 Message-ID: <83k267pws0.fsf@gnu.org> References: <20170422152444.48735-1-phst@google.com> <83zif8p8d3.fsf@gnu.org> <83tw5gp6hy.fsf@gnu.org> <83r30kp5d1.fsf@gnu.org> Reply-To: Eli Zaretskii NNTP-Posting-Host: blaine.gmane.org X-Trace: blaine.gmane.org 1493184292 19033 195.159.176.226 (26 Apr 2017 05:24:52 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Wed, 26 Apr 2017 05:24:52 +0000 (UTC) Cc: emacs-devel@gnu.org To: Stefan Monnier Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Apr 26 07:24:45 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1d3FRV-0004ir-3v for ged-emacs-devel@m.gmane.org; Wed, 26 Apr 2017 07:24:45 +0200 Original-Received: from localhost ([::1]:52692 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d3FRX-00072w-It for ged-emacs-devel@m.gmane.org; Wed, 26 Apr 2017 01:24:47 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:35058) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d3FQw-00072p-8i for emacs-devel@gnu.org; Wed, 26 Apr 2017 01:24:11 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d3FQt-00018y-3d for emacs-devel@gnu.org; Wed, 26 Apr 2017 01:24:10 -0400 Original-Received: from fencepost.gnu.org ([2001:4830:134:3::e]:49250) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d3FQt-00018u-0R; Wed, 26 Apr 2017 01:24:07 -0400 Original-Received: from 84.94.185.246.cable.012.net.il ([84.94.185.246]:3409 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1d3FQs-0007Ns-9o; Wed, 26 Apr 2017 01:24:06 -0400 In-reply-to: (message from Stefan Monnier on Sun, 23 Apr 2017 08:40:31 -0400) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2001:4830:134:3::e X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 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.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:214293 Archived-At: > From: Stefan Monnier > Date: Sun, 23 Apr 2017 08:40:31 -0400 > > EZ> I don't think I understand this. From my POV, restricting modules to be > EZ> called only from one thread is too restrictive, and I see no reason for > EZ> that. > > I see Eli's point here; I'm wondering Philipp, did you run into a particular > > problem your patch is trying to solve, or are you trying to preempt future > > problems? > > IIUC this doesn't restrict a module to be used with only one thread. > It just makes sure that the module can only call back Elisp from the > same thread that called it (and "called it" doesn't mean here just "some > time in the past" but "somewhere up the stack"). Isn't that guaranteed? IOW, what would be a scenario where in the situation you described we will be in a different thread? Perhaps we should make a step back and ask ourselves what is the purpose of this check. AFAIU, its original purpose was to make sure module functions are called only from the Lisp thread, not from some thread started by some linked-in library. Nowadays, we can have more than one Lisp thread, but the analogous check still makes sense, IMO. So how about this much simpler implementation: static void check_thread (void) { #ifdef THREADS_ENABLED if (current_thread) { # if defined (HAVE_PTHREAD) eassert (pthread_equal (pthread_self (), current_thread->thread_id)); # elif defined (WINDOWSNT) eassert (GetCurrentThreadId () == current_thread->thread_id); # endif } #endif } This fixes a few minor problems with the original proposal: . it works in an Emacs built without threads . it does NOT require current_thread to be non-NULL, as this can legitimately happen during short periods of time (see thread.c), and . it avoids recording some arbitrary thread in the env struct The main idea of the check is still there, and will do its job. Comments?