From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Pip Cet Newsgroups: gmane.emacs.bugs Subject: bug#21380: 25.0.50; GTK-induced segfault when scheduling timer from window-configuration-change-hook Date: Tue, 1 Sep 2015 10:20:11 +0000 Message-ID: References: <83mvx8252m.fsf@gnu.org> <83k2sc20k6.fsf@gnu.org> <83h9ng1ryx.fsf@gnu.org> <83a8t71qct.fsf@gnu.org> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary=001a1144ac342b7664051eace58f X-Trace: ger.gmane.org 1441102883 7958 80.91.229.3 (1 Sep 2015 10:21:23 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 1 Sep 2015 10:21:23 +0000 (UTC) Cc: 21380@debbugs.gnu.org To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Tue Sep 01 12:21:14 2015 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1ZWigk-0003Bk-8m for geb-bug-gnu-emacs@m.gmane.org; Tue, 01 Sep 2015 12:21:14 +0200 Original-Received: from localhost ([::1]:52310 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWigj-0002dv-KL for geb-bug-gnu-emacs@m.gmane.org; Tue, 01 Sep 2015 06:21:13 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:35109) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWigd-0002do-NJ for bug-gnu-emacs@gnu.org; Tue, 01 Sep 2015 06:21:09 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWigY-0007Lh-Tc for bug-gnu-emacs@gnu.org; Tue, 01 Sep 2015 06:21:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:52530) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWigY-0007LN-Pi for bug-gnu-emacs@gnu.org; Tue, 01 Sep 2015 06:21:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1ZWigY-0005nK-Ba for bug-gnu-emacs@gnu.org; Tue, 01 Sep 2015 06:21:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Pip Cet Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 01 Sep 2015 10:21:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 21380 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 21380-submit@debbugs.gnu.org id=B21380.144110281622207 (code B ref 21380); Tue, 01 Sep 2015 10:21:02 +0000 Original-Received: (at 21380) by debbugs.gnu.org; 1 Sep 2015 10:20:16 +0000 Original-Received: from localhost ([127.0.0.1]:44740 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1ZWifn-0005m5-5h for submit@debbugs.gnu.org; Tue, 01 Sep 2015 06:20:16 -0400 Original-Received: from mail-io0-f172.google.com ([209.85.223.172]:34196) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1ZWifk-0005lx-8E for 21380@debbugs.gnu.org; Tue, 01 Sep 2015 06:20:13 -0400 Original-Received: by ioeu67 with SMTP id u67so47704972ioe.1 for <21380@debbugs.gnu.org>; Tue, 01 Sep 2015 03:20:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=ae8wfFBO0pfhuzv4f6+mJzu9qyEfMZnP2nsbuIq4p8c=; b=SwhvPfFerZIA/HxgED4y3aX2bC0ctPEE1MyGq5F/Esld7MEvAsBxQjFAFeK0SLXzTC vu+GkhzjogEmDf6roP8RFGhS/zVnigURh6BrF2HehPXrA4U82gLJxxEfn5HkkG5oMuZN ayKg0j9DgPAyWGw8sBzimPrABSM9YRT0FnaXo1s5p8lZLfi8brcF6tysw3oL2Vx8Bffv VrFaNo+knXn957ROFCe0A1XpD3I1maIa2Xg04HOEIss05eHRR8+CCEexO0kp9DB9ZiY1 SUpeQX+AFfISDGL96msVgXf13UTrDAe1nnU8yW/JnhYt2rDbLZAZ/DfGobceyi3U9gh0 uxzQ== X-Received: by 10.107.47.97 with SMTP id j94mr28723647ioo.136.1441102811578; Tue, 01 Sep 2015 03:20:11 -0700 (PDT) Original-Received: by 10.79.78.66 with HTTP; Tue, 1 Sep 2015 03:20:11 -0700 (PDT) In-Reply-To: <83a8t71qct.fsf@gnu.org> X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.15 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 208.118.235.43 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.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.bugs:106037 Archived-At: --001a1144ac342b7664051eace58f Content-Type: multipart/alternative; boundary=001a1144ac342b765f051eace58d --001a1144ac342b765f051eace58d Content-Type: text/plain; charset=UTF-8 On Mon, Aug 31, 2015 at 2:31 PM, Eli Zaretskii wrote: > > Yes for this particular segfault. > > Can you show a patch that fixes the original segfault in your use > case? Attached. Note that either one of those changes should work. I'll test this patch some more using my original code and see whether it blows up. I'm afraid I lost track, what with all the different scenarios > and potential solutions being thrown at this. We should install the > fix, assuming it's clean. > I think we should fix three things: - concat shouldn't rely on its argument remaining unchanged in length - the timer list copy should happen with block_input/unblock_input wrapped around it - we shouldn't call do_pending_window_change from QUIT [already installed. Thanks, martin!] Any one of these is enough to prevent the original segfault. All but the second also prevent the bizarre-elisp-induced segfault I came up with later. And I'm perfectly happy for today with the number of hooks called from QUIT reduced by one, rather than insisting on reducing them to zero right away. > No* for similar segfaults that I think pose equally severe problems: if > any other function calls concat/copy-sequence on data that is modified by > window-configuration-change-hook, it should* still be possible to produce > the segfault. > > Emacs gives you enough rope to hang yourself; there's nothing new > here. We should strive to protect the Emacs internals so that they > won't cause segfaults, but in user code any bets are off, and "don't > do that" is a valid response to whoever does such things. > It's always good to know what the philosophy is behind the way the code works, so thank you for that, really. > So it wouldn't even be safe for window-configuration-change-hook to add a > timer to the timer list, because the outer frame might be in the middle of > creating a copy of the timer list for some Lisp code that hasn't blocked > input. (As in my example below) > > Futzing with timers from within some hooks is indeed fundamentally > dangerous. Well, doing anything from window-configuration-change-hook is dangerous. My idea was to schedule an immediate timer from it to get out of the danger zone to do the actual work, but that backfired... > But we should still try to minimize the probability of a > crash, especially when it's Emacs itself who makes the offending copy, > because people do dangerous things all the time, and expect them to > work. In this case, blocking input should do, I think. > I agree. > I really don't think QUIT should run any Lisp hooks, to be honest. > > I don't think this limitation could fly. It will disable a lot of > useful use patterns, and the outcry will be loud and clear. > Okay. > If I'm wrong and QUIT should be able to run Lisp hooks, concat needs to > be fixed not to rely on its argument's size being unchanged after the > make_sequence call. > > That can't do any harm, so let's do it, too. > Cool. > > As far as I can tell, that should be reproducible. Also as far as I can > tell, it's merely a matter of luck that an X resize doesn't happen at the > point where I interrupted the program to artificially trigger the segfault. > However, I admit that it is a separate issue, less likely to occur in > practice, and I'll open another bug for it if that's the way you prefer > things. > > But if input is blocked, as it would be in the case of copying > timer-list inside timer_check, the X events will not be acted upon, > and the problem will not happen, right? > Indeed, that relies on bizarre elisp code deliberately doing silly things... > IOW, the above situation is a case of a user shooting herself in the > foot by having that particular function in the hook and that > particular code that copies timer-list (which is an internal variable > unwise users should not touch). Am I right? > I think you are. I'm not sure whether the timer code in timer.el does anything to the timer list that might count as dangerous, but that's possibly the only legitimate Lisp user of timer-list. --001a1144ac342b765f051eace58d Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
On Mon, Aug 31, 2015 at 2:31 PM, Eli Zaretskii <eliz@gnu.org= > wrote:
> Yes for this particular segf= ault.

Can you show a patch that fixes the original segfault in your use case?

Attached. Note that either one of tho= se changes should work. I'll test this patch some more using my origina= l code and see whether it blows up.

I'm afraid I lost track, what with all the different scenarios and potential solutions being thrown at this.=C2=A0 We should install the fix, assuming it's clean.

I think w= e should fix three things:
=C2=A0- concat shouldn't rely = on its argument remaining unchanged in length
=C2=A0- the tim= er list copy should happen with block_input/unblock_input wrapped around it=
=C2=A0- we shouldn't call do_pending_window_change from = QUIT [already installed. Thanks, martin!]

Any one of thes= e is enough to prevent the original segfault. All but the second also preve= nt the bizarre-elisp-induced segfault I came up with later. And I'm per= fectly happy for today with the number of hooks called from QUIT reduced by= one, rather than insisting on reducing them to zero right away.

> No* for similar segfaults that I think pose equally severe problems: i= f any other function calls concat/copy-sequence on data that is modified by= window-configuration-change-hook, it should* still be possible to produce = the segfault.

Emacs gives you enough rope to hang yourself; there's nothing ne= w
here.=C2=A0 We should strive to protect the Emacs internals so that they won't cause segfaults, but in user code any bets are off, and "don= 't
do that" is a valid response to whoever does such things.

It's always good to know what the philosophy is= behind the way the code works, so thank you for that, really.

> So it wouldn't even be safe for window-configuration-change-hook t= o add a timer to the timer list, because the outer frame might be in the mi= ddle of creating a copy of the timer list for some Lisp code that hasn'= t blocked input. (As in my example below)

Futzing with timers from within some hooks is indeed fundamentally dangerous.

Well, doing anything from window= -configuration-change-hook is dangerous. My idea was to schedule an immedia= te timer from it to get out of the danger zone to do the actual work, but t= hat backfired...
=C2=A0
= But we should still try to minimize the probability of a
crash, especially when it's Emacs itself who makes the offending copy,<= br> because people do dangerous things all the time, and expect them to
work.=C2=A0 In this case, blocking input should do, I think.

I agree.

> I really don't think QUIT should run any Lisp hooks, to be honest.=

I don't think this limitation could fly.=C2=A0 It will disable a= lot of
useful use patterns, and the outcry will be loud and clear.

Okay.

> If I'm wrong and QUIT should be able to run Lisp hooks, concat nee= ds to be fixed not to rely on its argument's size being unchanged after= the make_sequence call.

That can't do any harm, so let's do it, too.

Cool.
=C2=A0
> As far as I can tell, that should be reproducible. Also as far as I ca= n tell, it's merely a matter of luck that an X resize doesn't happe= n at the point where I interrupted the program to artificially trigger the = segfault. However, I admit that it is a separate issue, less likely to occu= r in practice, and I'll open another bug for it if that's the way y= ou prefer things.

But if input is blocked, as it would be in the case of copying
timer-list inside timer_check, the X events will not be acted upon,
and the problem will not happen, right?

Indeed, that relies on bizarre elisp code deliberately do= ing silly things...
=C2=A0
IOW, the above situation is a case of a user shooting herself in the
foot by having that particular function in the hook and that
particular code that copies timer-list (which is an internal variable
unwise users should not touch).=C2=A0 Am I right?

I think you are. I&= #39;m not sure whether the timer code in timer.el does anything to the time= r list that might count as dangerous, but that's possibly the only legi= timate Lisp user of timer-list.
--001a1144ac342b765f051eace58d-- --001a1144ac342b7664051eace58f Content-Type: text/x-patch; charset=US-ASCII; name="0001-Fix-potential-race-conditions-bug-23380.patch" Content-Disposition: attachment; filename="0001-Fix-potential-race-conditions-bug-23380.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_ie17enh80 RnJvbSA3ZTE4OGNhN2IwZTUxYmY3ZmZkYTQxM2NlYmM1M2RhOWViZmUwY2Q2IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBQaGlsaXAgPHBpcGNldEBnbWFpbC5jb20+CkRhdGU6IFR1ZSwg MSBTZXAgMjAxNSAxMDoxMjozMSArMDAwMApTdWJqZWN0OiBbUEFUQ0hdIEZpeCBwb3RlbnRpYWwg cmFjZSBjb25kaXRpb25zIChidWcgIzIzMzgwKQoKICAgICAgICAqIGtleWJvYXJkLmMgKHRpbWVy X2NoZWNrKTogQ2FsbCBgYmxvY2tfaW5wdXQnIGFyb3VuZCB0aGUgY3JlYXRpb24KCW9mIHRoZSB0 ZW1wb3JhcnkgdGltZXIgbGlzdCBjb3B5LgoKCSogZm5zLmMgKGNvbmNhdCk6IERvbid0IGFzc3Vt ZSBhcmd1bWVudCBzaXplIHJlbWFpbnMgdW5jaGFuZ2VkCglhZnRlciBjYWxsIHRvIGBGbWFrZV9s aXN0Jy4KLS0tCiBzcmMvZm5zLmMgICAgICB8IDMgKysrCiBzcmMva2V5Ym9hcmQuYyB8IDIgKysK IDIgZmlsZXMgY2hhbmdlZCwgNSBpbnNlcnRpb25zKCspCgpkaWZmIC0tZ2l0IGEvc3JjL2Zucy5j IGIvc3JjL2Zucy5jCmluZGV4IDI2YTk4YWIuLjQzZmY1NTAgMTAwNjQ0Ci0tLSBhL3NyYy9mbnMu YworKysgYi9zcmMvZm5zLmMKQEAgLTc0NCw2ICs3NDQsOSBAQCBjb25jYXQgKHB0cmRpZmZfdCBu YXJncywgTGlzcF9PYmplY3QgKmFyZ3MsCiAJICAgIC8qIFN0b3JlIHRoaXMgZWxlbWVudCBpbnRv IHRoZSByZXN1bHQuICAqLwogCSAgICBpZiAodG9pbmRleCA8IDApCiAJICAgICAgeworICAgICAg ICAgICAgICAgIGlmIChFUSAodGFpbCwgUW5pbCkpCisgICAgICAgICAgICAgICAgICBicmVhazsK KwogCQlYU0VUQ0FSICh0YWlsLCBlbHQpOwogCQlwcmV2ID0gdGFpbDsKIAkJdGFpbCA9IFhDRFIg KHRhaWwpOwpkaWZmIC0tZ2l0IGEvc3JjL2tleWJvYXJkLmMgYi9zcmMva2V5Ym9hcmQuYwppbmRl eCBkYWIzMmIxLi40YjdkNjc1IDEwMDY0NAotLS0gYS9zcmMva2V5Ym9hcmQuYworKysgYi9zcmMv a2V5Ym9hcmQuYwpAQCAtNDU2MCw2ICs0NTYwLDcgQEAgdGltZXJfY2hlY2sgKHZvaWQpCiAKICAg TGlzcF9PYmplY3QgdGVtID0gVmluaGliaXRfcXVpdDsKICAgVmluaGliaXRfcXVpdCA9IFF0Owor ICBibG9ja19pbnB1dCAoKTsKIAogICAvKiBXZSB1c2UgY29waWVzIG9mIHRoZSB0aW1lcnMnIGxp c3RzIHRvIGFsbG93IGEgdGltZXIgdG8gYWRkIGl0c2VsZgogICAgICBhZ2Fpbiwgd2l0aG91dCBs b2NraW5nIHVwIEVtYWNzIGlmIHRoZSBuZXdseSBhZGRlZCB0aW1lciBpcwpAQCAtNDU3Myw2ICs0 NTc0LDcgQEAgdGltZXJfY2hlY2sgKHZvaWQpCiAgIGVsc2UKICAgICBpZGxlX3RpbWVycyA9IFFu aWw7CiAKKyAgdW5ibG9ja19pbnB1dCAoKTsKICAgVmluaGliaXRfcXVpdCA9IHRlbTsKIAogICBk bwotLSAKMi41LjAKCg== --001a1144ac342b7664051eace58f--