From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Philipp Stephani Newsgroups: gmane.emacs.devel Subject: Re: [PATCH] Support threads in modules Date: Sat, 10 Jun 2017 19:58:40 +0000 Message-ID: References: <20170422152444.48735-1-phst@google.com> <83zif8p8d3.fsf@gnu.org> <83tw5gp6hy.fsf@gnu.org> <83r30kp5d1.fsf@gnu.org> <83inlrpwds.fsf@gnu.org> <8337b86mrl.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="001a113cd192c560bd0551a083b5" X-Trace: blaine.gmane.org 1497124757 23570 195.159.176.226 (10 Jun 2017 19:59:17 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 10 Jun 2017 19:59:17 +0000 (UTC) Cc: phst@google.com, emacs-devel@gnu.org To: Eli Zaretskii Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Jun 10 21:59:11 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 1dJmXN-0005Xa-6S for ged-emacs-devel@m.gmane.org; Sat, 10 Jun 2017 21:59:09 +0200 Original-Received: from localhost ([::1]:60040 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dJmXP-0007pj-AH for ged-emacs-devel@m.gmane.org; Sat, 10 Jun 2017 15:59:11 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:33153) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dJmXC-0007oP-Md for emacs-devel@gnu.org; Sat, 10 Jun 2017 15:59:00 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dJmXA-0006XY-M2 for emacs-devel@gnu.org; Sat, 10 Jun 2017 15:58:58 -0400 Original-Received: from mail-oi0-x22d.google.com ([2607:f8b0:4003:c06::22d]:32911) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dJmX7-0006WS-Qp; Sat, 10 Jun 2017 15:58:54 -0400 Original-Received: by mail-oi0-x22d.google.com with SMTP id s3so41178045oia.0; Sat, 10 Jun 2017 12:58:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=NgpcrS3QdRpYK1j8g+LmFmlV4qTwA60c7zQRR0G5Qhw=; b=rMqXu6cb1ouSeGJ/PjuETzMN9ZnIdivR776zV7GVdPqNi5pk53B7MnJXu2szXAT2Vz fhX+spIlNLYOoc5FUuDHcAluc44gsxQKPzPRDhMrvf3ptfPBXH5GG/RmDn1GUNn7+XCZ UYz7LLPwLba1HiNvonxzTCEBJfwho4X7qPA4fZPIdzWf4SMuWdHXvZasO7BsPkCyKhOB O6Wz6/NSfyXojWxn2thJf1E8fsoaMHKXYXdmaBV3kKgSi8nJi4j1dSuuyXdiwW0/LNZc bZmUAmb6L9NjhBkWpUBxDufD++FynkarMQycbARbpgLUC+aS1isiNrET9pgwFZ+aPDse 1EPA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=NgpcrS3QdRpYK1j8g+LmFmlV4qTwA60c7zQRR0G5Qhw=; b=pbYCxcGRXErfhVoKGCc85sKYMJ0wkwqFvMfji7UzNKCb1RxChkqqiryVwfOW3wq0mG sEsiqvhOJ0W12w2c4n4T4a5iAkIUBy3fKnYq2pZc/oAbVkRDFApfJWVBxhAYh2EqW8kV 3rrje7+KFZy4fuIlfkaIhq8fvHN62rbs3s5xKy6J+iXPj5ompZfBQvjE6za2BqRfYzLs 0stEOzPyHlMEUrJke+QzkZRtIU8lMEu8alyhm6sO7yrxLpIDTljHdD90o40G7PJ1WEpV 7N9QPMBGuWGedeN0Hi0MIo8mobsrLLkF8pojglTHYra2rzVHaOgNBArREW1Awbmh/EQD KWkQ== X-Gm-Message-State: AODbwcBmBm/Db7zFo6rfwoC0aaVBzKnQPl+M3MaDcsZUDZAlJ8Ne52VF dQ0iUFwwAAC1sXhT2R6eBozOWnBpPdkU X-Received: by 10.202.51.7 with SMTP id z7mr20865607oiz.75.1497124730919; Sat, 10 Jun 2017 12:58:50 -0700 (PDT) In-Reply-To: <8337b86mrl.fsf@gnu.org> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:4003:c06::22d 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:215567 Archived-At: --001a113cd192c560bd0551a083b5 Content-Type: text/plain; charset="UTF-8" Eli Zaretskii schrieb am Sa., 10. Juni 2017 um 14:38 Uhr: > > From: Philipp Stephani > > Date: Sat, 10 Jun 2017 11:38:32 +0000 > > Cc: emacs-devel@gnu.org, phst@google.com > > > > Eli Zaretskii schrieb am Mi., 26. Apr. 2017 um 07:32 Uhr: > > [It's very hard to have a discussion with 1.5 months between messages.] > Yes, sorry for the huge delay, I wanted to follow up here for a while :-/ > > > > > > - Using objects across threads requires careful synchronization. > > > > > > > > Not sure what synchronization you have in mind. There's only one > > > > running thread at any given time, and a thread cannot be preempted > > > > while it runs Lisp, so synchronization seems unnecessary, as a > thread > > > > cannot modify a Lisp object that another thread is modifying. > > > > > > > > However, that's just an implementation detail, and the mapping > between > > > Emacs and OS threads is > > > > unspecified (for good reason). > > > > > > It isn't unspecified: the ELisp manual explicitly documents that only > > > one Lisp thread can run at any given time. > > > > That is true for a *Lisp* thread, not for an *OS* thread. On the OS > thread > > level we might theoretically have multiple threads per Lisp thread, or > miss > > some synchronization barriers, etc. > > You were AFAIU talking about accesses to Lisp objects, and these are > only possible via Lisp, which can only be run in a single thread at a > time. So if the non-Lisp threads of any kind can enter this picture, > in a way that module functions will be run by those threads and access > Lisp objects, please describe such a use case, so we all are on the > same page regarding the situations we are considering. > Such a use case is what these assertions should guard against. Calling environment functions from non-Lisp threads is undefined behavior, and these assertions are meant to protect module developers against them. > > > > > To be forward-compatible, either we have to formally document this > and > > > then > > > > never ever change the implementation, or document that module authors > > > can't rely on it and have to perform > > > > synchronization themselves. > > > > > > Since it is documented already, module authors can rely on that. > > > > It's not documented because it can't be documented without talking about > > the behavior of the C module API, and that is itself not documented yet. > > If what you mean is that we don't say explicitly in the doc that only > one Lisp thread can be active at any given time, we can add that > statement right now. Would that be enough to settle this particular > argument. > There is no actual argument :-) It's just a matter of how much documentation we need. I think the guaranteed synchronization should be documented in the to-be-written modules manual as well, if we choose to make the guarantee. > > > If > > we choose to guarantee that unsynchronized accesses to module functions > > don't introduce data races, then we need to say that explicitly in the > > module documentation, and stick to it. (I.e. once that decision has been > > made, there's no going back.) > > I think that ship sailed long ago: the current implementation assumes > implicitly and explicitly that at most only one Lisp thread runs at > any given time. If we will ever want to have an implementation that > violates this constraint, we will have to completely rewrite > everything that is related to threads, including emacs-modules.c. > Yes, but if we do decide to rewrite, then the new code might need additional synchronization if we guarantee thread safety which would otherwise not be required. > > > > > I meant this as a general statement: if an API is more restrictive > (i.e. > > > has stronger preconditions), its > > > > implementation can be more flexible because it has to deal with fewer > > > corner cases. > > > > > > I don't think it's right for us to favor our flexibility over that of > > > the Lisp programmers who will want to use these features. > > > > That's a matter for debate. > > I'm saying that as long as this is debatable, we shouldn't make life > for Lisp programmers so much harder based on assumptions that are not > universally approved by the project as a whole. My opinion is that > the Lisp programmers' flexibility should be preferable to that of > developers. > The two options are: 1. Guarantee thread safety of module objects. 2. Don't guarantee thread safety of module objects. You clearly advocate for (1), which is fine. What I'm trying to point out is that if we picked (2) now, we can easily switch to (1) later, but not the other way round. Once we decide to do (1), there's no way to go to (2), even if in some future implementation (2) might become simpler or more efficient. That's the tradeoff. > > > > I think we only care that module functions run in the context of some > > > Lisp thread, and we don't care which Lisp thread is that. So I > > > proposed a simplified implementation of the test, which I hope you > > > will agree with. > > > > If we want to guarantee that environment functions can be called from > > arbitrary threads without introducing data races, then the only assertion > > that's necessary is whether (current_thread->thread_id == > > GetCurrentThreadId ()); without that undefined behavior would be almost > > guaranteed. > > Sorry, I don't understand what you are saying here. Especially since > your proposed condition is almost the one I proposed in > > http://lists.gnu.org/archive/html/emacs-devel/2017-04/msg00720.html > > Does this mean that you agree with my reasoning and the code I > proposed there? > Yes, almost, if we pick option (1) now. > > Once again: there _are_ legitimate situations in Emacs when for a > short time current_thread is NULL. If you assume that these > situations don't happen, your code will sooner or later crash and > burn. I'm saying this because I once thought current_thread should be > non-NULL at all times, and my code which assumed that did crash. > I've reviewed the threads code, and I'm quite sure that current_thread can never be NULL during check_thread. current_thread is only NULL between exiting a Lisp thread and resuming execution in another thread after reacquiring the GIL. The evaluator doesn't run during that phase, and therefore no module functions can run. current_thread could only be NULL during check_thread if called from a thread that is not a Lisp thread (i.e. an OS thread created by the module). That's exactly one of the undefined behavior situations that the assertions are meant to prevent. --001a113cd192c560bd0551a083b5 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


Eli Za= retskii <eliz@gnu.org> schrieb am= Sa., 10. Juni 2017 um 14:38=C2=A0Uhr:
> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 10 Jun 2017 11:38:32 +0000
> Cc: emacs-dev= el@gnu.org, phst@g= oogle.com
>
> Eli Zaretskii <el= iz@gnu.org> schrieb am Mi., 26. Apr. 2017 um 07:32 Uhr:

[It's very hard to have a discussion with 1.5 months between messages.]=

Yes, sorry for the huge delay, I wante= d to follow up here for a while :-/
=C2=A0

> > >=C2=A0 > - Using objects across threads requires careful s= ynchronization.
> > >
> > >=C2=A0 Not sure what synchronization you have in mind. There&= #39;s only one
> > >=C2=A0 running thread at any given time, and a thread cannot = be preempted
> > >=C2=A0 while it runs Lisp, so synchronization seems unnecessa= ry, as a thread
> > >=C2=A0 cannot modify a Lisp object that another thread is mod= ifying.
> > >
> > > However, that's just an implementation detail, and the m= apping between
> > Emacs and OS threads is
> > > unspecified (for good reason).
> >
> > It isn't unspecified: the ELisp manual explicitly documents t= hat only
> > one Lisp thread can run at any given time.
>
> That is true for a *Lisp* thread, not for an *OS* thread. On the OS th= read
> level we might theoretically have multiple threads per Lisp thread, or= miss
> some synchronization barriers, etc.

You were AFAIU talking about accesses to Lisp objects, and these are
only possible via Lisp, which can only be run in a single thread at a
time.=C2=A0 So if the non-Lisp threads of any kind can enter this picture,<= br> in a way that module functions will be run by those threads and access
Lisp objects, please describe such a use case, so we all are on the
same page regarding the situations we are considering.

Such a use case is what these assertions should guard again= st. Calling environment functions from non-Lisp threads is undefined behavi= or, and these assertions are meant to protect module developers against the= m.
=C2=A0

> > > To be forward-compatible, either we have to formally documen= t this and
> > then
> > > never ever change the implementation, or document that modul= e authors
> > can't rely on it and have to perform
> > > synchronization themselves.
> >
> > Since it is documented already, module authors can rely on that.<= br> >
> It's not documented because it can't be documented without tal= king about
> the behavior of the C module API, and that is itself not documented ye= t.

If what you mean is that we don't say explicitly in the doc that only one Lisp thread can be active at any given time, we can add that
statement right now.=C2=A0 Would that be enough to settle this particular argument.

There is no actual argument := -)
It's just a matter of how much documentation we need. I th= ink the guaranteed synchronization should be documented in the to-be-writte= n modules manual as well, if we choose to make the guarantee.
=C2= =A0

> If
> we choose to guarantee that unsynchronized accesses to module function= s
> don't introduce data races, then we need to say that explicitly in= the
> module documentation, and stick to it. (I.e. once that decision has be= en
> made, there's no going back.)

I think that ship sailed long ago: the current implementation assumes
implicitly and explicitly that at most only one Lisp thread runs at
any given time.=C2=A0 If we will ever want to have an implementation that violates this constraint, we will have to completely rewrite
everything that is related to threads, including emacs-modules.c.

Yes, but if we do decide to rewrite, then the ne= w code might need additional synchronization if we guarantee thread safety = which would otherwise not be required.
=C2=A0

> > > I meant this as a general statement: if an API is more restr= ictive (i.e.
> > has stronger preconditions), its
> > > implementation can be more flexible because it has to deal w= ith fewer
> > corner cases.
> >
> > I don't think it's right for us to favor our flexibility = over that of
> > the Lisp programmers who will want to use these features.
>
> That's a matter for debate.

I'm saying that as long as this is debatable, we shouldn't make lif= e
for Lisp programmers so much harder based on assumptions that are not
universally approved by the project as a whole.=C2=A0 My opinion is that the Lisp programmers' flexibility should be preferable to that of
developers.

The two options are:
<= div>1. Guarantee thread safety of module objects.
2. Don't gu= arantee thread safety of module objects.
You clearly advocate for= (1), which is fine. What I'm trying to point out is that if we picked = (2) now, we can easily switch to (1) later, but not the other way round. On= ce we decide to do (1), there's no way to go to (2), even if in some fu= ture implementation (2) might become simpler or more efficient. That's = the tradeoff.
=C2=A0

> > I think we only care that module functions run in the context of = some
> > Lisp thread, and we don't care which Lisp thread is that.=C2= =A0 So I
> > proposed a simplified implementation of the test, which I hope yo= u
> > will agree with.
>
> If we want to guarantee that environment functions can be called from<= br> > arbitrary threads without introducing data races, then the only assert= ion
> that's necessary is whether (current_thread->thread_id =3D=3D > GetCurrentThreadId ()); without that undefined behavior would be almos= t
> guaranteed.

Sorry, I don't understand what you are saying here.=C2=A0 Especially si= nce
your proposed condition is almost the one I proposed in

=C2=A0 http://lists.gnu.org/archi= ve/html/emacs-devel/2017-04/msg00720.html

Does this mean that you agree with my reasoning and the code I
proposed there?

Yes, almost, if we pick= option (1) now.
=C2=A0

Once again: there _are_ legitimate situations in Emacs when for a
short time current_thread is NULL.=C2=A0 If you assume that these
situations don't happen, your code will sooner or later crash and
burn.=C2=A0 I'm saying this because I once thought current_thread shoul= d be
non-NULL at all times, and my code which assumed that did crash.

I've reviewed the threads code, and I'm q= uite sure that current_thread can never be NULL during check_thread. curren= t_thread is only NULL between exiting a Lisp thread and resuming execution = in another thread after reacquiring the GIL. The evaluator doesn't run = during that phase, and therefore no module functions can run.
cur= rent_thread could only be NULL during check_thread if called from a thread = that is not a Lisp thread (i.e. an OS thread created by the module). That&#= 39;s exactly one of the undefined behavior situations that the assertions a= re meant to prevent.

--001a113cd192c560bd0551a083b5--