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 15:14:09 +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=001a1144ac34852304051eb10037 X-Trace: ger.gmane.org 1441120524 10656 80.91.229.3 (1 Sep 2015 15:15:24 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 1 Sep 2015 15:15:24 +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 17:15:15 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 1ZWnHF-0000Pg-LL for geb-bug-gnu-emacs@m.gmane.org; Tue, 01 Sep 2015 17:15:13 +0200 Original-Received: from localhost ([::1]:54976 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWnHF-0007Tw-Ee for geb-bug-gnu-emacs@m.gmane.org; Tue, 01 Sep 2015 11:15:13 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:60655) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWnH8-0007RJ-Jj for bug-gnu-emacs@gnu.org; Tue, 01 Sep 2015 11:15:08 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZWnH5-0000ca-4I for bug-gnu-emacs@gnu.org; Tue, 01 Sep 2015 11:15:06 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:53015) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZWnH5-0000cO-2C for bug-gnu-emacs@gnu.org; Tue, 01 Sep 2015 11:15:03 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1ZWnH4-00067a-Ll for bug-gnu-emacs@gnu.org; Tue, 01 Sep 2015 11:15: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 15:15: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.144112045623457 (code B ref 21380); Tue, 01 Sep 2015 15:15:02 +0000 Original-Received: (at 21380) by debbugs.gnu.org; 1 Sep 2015 15:14:16 +0000 Original-Received: from localhost ([127.0.0.1]:45225 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1ZWnGJ-00066G-8z for submit@debbugs.gnu.org; Tue, 01 Sep 2015 11:14:16 -0400 Original-Received: from mail-io0-f170.google.com ([209.85.223.170]:35121) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1ZWnGG-000663-Cs for 21380@debbugs.gnu.org; Tue, 01 Sep 2015 11:14:13 -0400 Original-Received: by ioiz6 with SMTP id z6so4531011ioi.2 for <21380@debbugs.gnu.org>; Tue, 01 Sep 2015 08:14: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=y8yohLojjDk7OJqePZTgBsjwQolYnFaUAJtC9GdlwU8=; b=eWcoYvO894P75tjzIYh9kkA3ETNdjDhQEeUB9i78AGKcVflUKqyDHFgSRA5uTrBYNh Cj0zQBWNa7KNzYbcKagrylX0jwYyrSXCR7zUfzjbi7VOHgAcw9NIk3dDRg5PuU/qjLoD TYl5fdolrLIvR4AW/ek91UKyyrlQGEcQ73XpL9Swp2VOdr/bPZ43f20/Z5/BJvV32wjq CneSWzeVil8HeMIeMOVSDbLOUR/mCd0C/qb3y21/hNwlsIn+YXkgjy+Nuzg19S0kokNQ 2pggilKVygtmQRAuM8p95qA+sfTgM5bF4EZ4oTpq/NdHSaKhftSVuN/Da0WdfqVGmQyY gwNg== X-Received: by 10.107.47.97 with SMTP id j94mr30020429ioo.136.1441120450292; Tue, 01 Sep 2015 08:14:10 -0700 (PDT) Original-Received: by 10.79.78.66 with HTTP; Tue, 1 Sep 2015 08:14:09 -0700 (PDT) In-Reply-To: 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:106045 Archived-At: --001a1144ac34852304051eb10037 Content-Type: multipart/alternative; boundary=001a1144ac34852300051eb10035 --001a1144ac34852300051eb10035 Content-Type: text/plain; charset=UTF-8 No segfault*. (* - well, one segfault. But I attribute that to extraordinarily bizarre actions even by my standards: attempting to display an unprintable ASCII control character in the echo area. It seems we never make sure that non-standard faces are cached when redisplaying the echo area. Usually this is fine because propertized strings never end up in the echo area (I hope)...) Revised patch attached (use NILP, fix bug number in changelog entry). On Tue, Sep 1, 2015 at 10:20 AM, Pip Cet wrote: > 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. > --001a1144ac34852300051eb10035 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
No segfault*.

(* - well, one segfault. But I a= ttribute that to extraordinarily bizarre actions even by my standards: atte= mpting to display an unprintable ASCII control character in the echo area. = It seems we never make sure that non-standard faces are cached when redispl= aying the echo area. Usually this is fine because propertized strings never= end up in the echo area (I hope)...)

Revised patch attac= hed (use NILP, fix bug number in changelog entry).

=

On Tue, Sep= 1, 2015 at 10:20 AM, Pip Cet <pipcet@gmail.com> wrote:
On Mon, Aug = 31, 2015 at 2:31 PM, Eli Zaretskii <eliz@gnu.org> 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 th= e different scenarios
and potential solutions being thrown at this.=C2=A0 We should install the fix, assuming it's clean.

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

Any one = of these is enough to prevent the original segfault. All but the second als= o prevent the bizarre-elisp-induced segfault I came up with later. And I= 9;m perfectly happy for today with the number of hooks called from QUIT red= uced 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 philos= ophy 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 = immediate timer from it to get out of the danger zone to do the actual work= , but that 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 delibera= tely doing 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'm not sure whether the timer code in timer.el does anything to t= he timer list that might count as dangerous, but that's possibly the on= ly legitimate Lisp user of timer-list.

--001a1144ac34852300051eb10035-- --001a1144ac34852304051eb10037 Content-Type: text/x-patch; charset=US-ASCII; name="0001-Fix-potential-race-conditions-Bug-21380.patch" Content-Disposition: attachment; filename="0001-Fix-potential-race-conditions-Bug-21380.patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_ie1hwtdn0 RnJvbSA3ZGM4YzlkN2E0OTk3NzgxYzA0MDAxMzcxYWMzODRkYjI0YzM3ZTU5IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBQaGlsaXAgPHBpcGNldEBnbWFpbC5jb20+CkRhdGU6IFR1ZSwg MSBTZXAgMjAxNSAxMDoxMjozMSArMDAwMApTdWJqZWN0OiBbUEFUQ0hdIEZpeCBwb3RlbnRpYWwg cmFjZSBjb25kaXRpb25zIChCdWcjMjEzODApCgogICAgICAgICoga2V5Ym9hcmQuYyAodGltZXJf Y2hlY2spOiBDYWxsIGBibG9ja19pbnB1dCcgYXJvdW5kIHRoZSBjcmVhdGlvbgoJb2YgdGhlIHRl bXBvcmFyeSB0aW1lciBsaXN0IGNvcHkuCgoJKiBmbnMuYyAoY29uY2F0KTogRG9uJ3QgYXNzdW1l IGFyZ3VtZW50IHNpemUgcmVtYWlucyB1bmNoYW5nZWQKCWFmdGVyIGNhbGwgdG8gYEZtYWtlX2xp c3QnLgotLS0KIHNyYy9mbnMuYyAgICAgIHwgMyArKysKIHNyYy9rZXlib2FyZC5jIHwgMiArKwog MiBmaWxlcyBjaGFuZ2VkLCA1IGluc2VydGlvbnMoKykKCmRpZmYgLS1naXQgYS9zcmMvZm5zLmMg Yi9zcmMvZm5zLmMKaW5kZXggMjZhOThhYi4uMTVkOWU2NCAxMDA2NDQKLS0tIGEvc3JjL2Zucy5j CisrKyBiL3NyYy9mbnMuYwpAQCAtNzQ0LDYgKzc0NCw5IEBAIGNvbmNhdCAocHRyZGlmZl90IG5h cmdzLCBMaXNwX09iamVjdCAqYXJncywKIAkgICAgLyogU3RvcmUgdGhpcyBlbGVtZW50IGludG8g dGhlIHJlc3VsdC4gICovCiAJICAgIGlmICh0b2luZGV4IDwgMCkKIAkgICAgICB7CisgICAgICAg ICAgICAgICAgaWYgKE5JTFAgKHRhaWwpKQorICAgICAgICAgICAgICAgICAgYnJlYWs7CisKIAkJ WFNFVENBUiAodGFpbCwgZWx0KTsKIAkJcHJldiA9IHRhaWw7CiAJCXRhaWwgPSBYQ0RSICh0YWls KTsKZGlmZiAtLWdpdCBhL3NyYy9rZXlib2FyZC5jIGIvc3JjL2tleWJvYXJkLmMKaW5kZXggZGFi MzJiMS4uNGI3ZDY3NSAxMDA2NDQKLS0tIGEvc3JjL2tleWJvYXJkLmMKKysrIGIvc3JjL2tleWJv YXJkLmMKQEAgLTQ1NjAsNiArNDU2MCw3IEBAIHRpbWVyX2NoZWNrICh2b2lkKQogCiAgIExpc3Bf T2JqZWN0IHRlbSA9IFZpbmhpYml0X3F1aXQ7CiAgIFZpbmhpYml0X3F1aXQgPSBRdDsKKyAgYmxv Y2tfaW5wdXQgKCk7CiAKICAgLyogV2UgdXNlIGNvcGllcyBvZiB0aGUgdGltZXJzJyBsaXN0cyB0 byBhbGxvdyBhIHRpbWVyIHRvIGFkZCBpdHNlbGYKICAgICAgYWdhaW4sIHdpdGhvdXQgbG9ja2lu ZyB1cCBFbWFjcyBpZiB0aGUgbmV3bHkgYWRkZWQgdGltZXIgaXMKQEAgLTQ1NzMsNiArNDU3NCw3 IEBAIHRpbWVyX2NoZWNrICh2b2lkKQogICBlbHNlCiAgICAgaWRsZV90aW1lcnMgPSBRbmlsOwog CisgIHVuYmxvY2tfaW5wdXQgKCk7CiAgIFZpbmhpYml0X3F1aXQgPSB0ZW07CiAKICAgZG8KLS0g CjIuNS4wCgo= --001a1144ac34852304051eb10037--