From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Dima Kogan Newsgroups: gmane.emacs.bugs Subject: bug#18861: Acknowledgement (25.0.50; gfile-based file notifications are not immediate) Date: Wed, 05 Nov 2014 11:19:46 -0800 Message-ID: <877fz9cv12.fsf@secretsauce.net> References: <87zjchdokj.fsf@secretsauce.net> <87wq7kes7w.fsf@secretsauce.net> <87vbn3eup2.fsf@secretsauce.net> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: ger.gmane.org 1415215402 8093 80.91.229.3 (5 Nov 2014 19:23:22 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 5 Nov 2014 19:23:22 +0000 (UTC) Cc: 18861@debbugs.gnu.org To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Nov 05 20:23:17 2014 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 1Xm6Am-00074c-AT for geb-bug-gnu-emacs@m.gmane.org; Wed, 05 Nov 2014 20:23:16 +0100 Original-Received: from localhost ([::1]:48168 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xm6Al-0004JE-TX for geb-bug-gnu-emacs@m.gmane.org; Wed, 05 Nov 2014 14:23:15 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:35889) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xm6Ae-0004I5-2X for bug-gnu-emacs@gnu.org; Wed, 05 Nov 2014 14:23:12 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xm6AY-0005fJ-Ti for bug-gnu-emacs@gnu.org; Wed, 05 Nov 2014 14:23:08 -0500 Original-Received: from debbugs.gnu.org ([140.186.70.43]:53151) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xm6AY-0005f4-HW for bug-gnu-emacs@gnu.org; Wed, 05 Nov 2014 14:23:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.80) (envelope-from ) id 1Xm6AY-0007Dr-0R for bug-gnu-emacs@gnu.org; Wed, 05 Nov 2014 14:23:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Dima Kogan Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 05 Nov 2014 19:23:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 18861 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 18861-submit@debbugs.gnu.org id=B18861.141521534227704 (code B ref 18861); Wed, 05 Nov 2014 19:23:01 +0000 Original-Received: (at 18861) by debbugs.gnu.org; 5 Nov 2014 19:22:22 +0000 Original-Received: from localhost ([127.0.0.1]:50364 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xm69u-0007Cl-0m for submit@debbugs.gnu.org; Wed, 05 Nov 2014 14:22:22 -0500 Original-Received: from out1-smtp.messagingengine.com ([66.111.4.25]:56388) by debbugs.gnu.org with esmtp (Exim 4.80) (envelope-from ) id 1Xm69r-0007Cd-7e for 18861@debbugs.gnu.org; Wed, 05 Nov 2014 14:22:20 -0500 Original-Received: from compute2.internal (compute2.nyi.internal [10.202.2.42]) by mailout.nyi.internal (Postfix) with ESMTP id C7CF020B08 for <18861@debbugs.gnu.org>; Wed, 5 Nov 2014 14:22:18 -0500 (EST) Original-Received: from frontend1 ([10.202.2.160]) by compute2.internal (MEProxy); Wed, 05 Nov 2014 14:22:18 -0500 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=secretsauce.net; h=x-sasl-enc:references:from:to:cc:subject:date:in-reply-to :message-id:mime-version:content-type; s=mesmtp; bh=+2FltSNkQ4QC FZlVXLLt3kM1Rq0=; b=Dh3keaAkAk2ofaq0ioWkIZ+J6KL1d2MLXHvG2OB7/WFg GOf4/R5tZirChx/gw+HAyuy2SXhduCgUibCJIkGjywVofgcI1gnp1c4IVIAE2NX8 nBnO9tMKSlOaK82oWR4oViMLzp4a7P4jnKRIvP0902NtI8TTNcIvK7pOKfF8lwg= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=x-sasl-enc:references:from:to:cc:subject :date:in-reply-to:message-id:mime-version:content-type; s= smtpout; bh=+2FltSNkQ4QCFZlVXLLt3kM1Rq0=; b=puIMHaMQTl6QcspYFQiA UcrHqKPo07x9BbZmBLmSnz7ONBQ5XcP5q30dKinFO2FcGeNRPU+YydfNR4gAooQt fuCotRnPSukED0gwukwwWknw55OcO3OMMJ7CS1ZV7JTQEzlrjUFjZEA98dKVpq0U s03iHPaelh6jYiMYajCZtJY= X-Sasl-enc: /1oFDHiJP/eheWAp5EM4ZTi/rP2zdkI3aJ5X5g5Lqr7X 1415215338 Original-Received: from shorty.local (unknown [76.91.145.213]) by mail.messagingengine.com (Postfix) with ESMTPA id 817ACC0000E; Wed, 5 Nov 2014 14:22:18 -0500 (EST) Original-Received: from dima by shorty.local with local (Exim 4.82_1-5b7a7c0-XX) (envelope-from ) id 1Xm69p-0007yR-B9; Wed, 05 Nov 2014 11:22:17 -0800 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: 140.186.70.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:95566 --=-=-= Content-Type: text/plain Stefan Monnier writes: >> Hi. A glib maintainer responded to the bug report, and it turns out >> emacs was using glib slightly incorrectly. I'm attaching a patch to fix >> the issue, as suggested by the maintainer. > > Thanks. This is not my area of expertise at all, but since it seems > that noone else has replied yet, I'll try to move it along. > >> + if( g_main_context_acquire(context) != TRUE ) >> + { >> + // we couldn't acquire the context. I let this function proceed because it >> + // handles more than just glib file descriptors >> + retval = -1; >> + } >> + else >> + context_acquired = true; > > [ Please follow our coding conventions: put a space before open parens > (and not after, nor before close parens); use /*...*/ for comments; > capitalize and punctuate comments; use two spaces between sentences. > Also, I think "!g_main_context_acquire (context)" would be cleaner than > "g_main_context_acquire (context) != TRUE". I find comparing to NULL, > true, or false generally ugly, tho maybe that's just a personal taste > of mine that others don't share. ] > > Why set retval to -1? It seems safer to leave it unchanged (ie. set to 0). OK. How about the attached? The error-handling logic is better, and we don't touch glib if we couldn't acquire the context. However in this patch if we COULDN'T talk to glib, but we COULD talk to our own file descriptors, then xg_select() returns success, and there is no error reporting. Probably not what we want. --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-xg_select-now-acquires-the-glib-context-before-query.patch >From 1fcced5536c8d99032303fb0f547aa67a5cccd30 Mon Sep 17 00:00:00 2001 From: Dima Kogan Date: Tue, 28 Oct 2014 14:29:01 -0700 Subject: [PATCH] xg_select() now acquires the glib context before querying it Prior to this patch we were calling g_main_context_query() without calling g_main_context_acquire(). This resulted in the file descriptors returned by g_main_context_query() missing activity. I.e. something would happen in glib, but a select() on the file descriptors would keep blocking. We now acquire the context, which makes select() return on activity, as it should. This is emacs and glib bugs: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=18861 https://bugzilla.gnome.org/show_bug.cgi?id=739274 --- src/xgselect.c | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/xgselect.c b/src/xgselect.c index bf889a9..7dee38d 100644 --- a/src/xgselect.c +++ b/src/xgselect.c @@ -55,19 +55,28 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, fd_set *efds, GPollFD *gfds = gfds_buf; int gfds_size = ARRAYELTS (gfds_buf); int n_gfds, retval = 0, our_fds = 0, max_fds = fds_lim - 1; - int i, nfds, tmo_in_millisec; + bool context_acquired = false; + int i, nfds, tmo_in_millisec = -1; bool need_to_dispatch; USE_SAFE_ALLOCA; + /* If we couldn't acquire the context, I let this function proceed because it + handles more than just glib file descriptors. Note that, as implemented, + this failure is completely silent: there is no feedback to the caller. */ context = g_main_context_default (); + if (g_main_context_acquire(context)) + context_acquired = true; if (rfds) all_rfds = *rfds; else FD_ZERO (&all_rfds); if (wfds) all_wfds = *wfds; else FD_ZERO (&all_wfds); - n_gfds = g_main_context_query (context, G_PRIORITY_LOW, &tmo_in_millisec, - gfds, gfds_size); + n_gfds = context_acquired ? + g_main_context_query (context, G_PRIORITY_LOW, &tmo_in_millisec, + gfds, gfds_size) : + -1; + if (gfds_size < n_gfds) { SAFE_NALLOCA (gfds, sizeof *gfds, n_gfds); @@ -152,6 +161,9 @@ xg_select (int fds_lim, fd_set *rfds, fd_set *wfds, fd_set *efds, errno = pselect_errno; } + if (context_acquired) + g_main_context_release(context); + /* To not have to recalculate timeout, return like this. */ if ((our_fds > 0 || (nfds == 0 && tmop == &tmo)) && (retval == 0)) { -- 2.0.0 --=-=-=--