From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) Subject: bug#28784: Simplifying revealed ports Date: Wed, 11 Oct 2017 21:41:21 +0200 Message-ID: <871sm9bhge.fsf__30547.3052700565$1507750943$gmane$org@gnu.org> References: <8760bldi1y.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:37073) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1e2MtI-00072c-9H for bug-guix@gnu.org; Wed, 11 Oct 2017 15:42:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1e2MtG-0007Pv-Vh for bug-guix@gnu.org; Wed, 11 Oct 2017 15:42:04 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:54039) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1e2MtG-0007PV-Rr for bug-guix@gnu.org; Wed, 11 Oct 2017 15:42:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1e2MtG-0005u2-Ig for bug-guix@gnu.org; Wed, 11 Oct 2017 15:42:02 -0400 In-Reply-To: <8760bldi1y.fsf@gnu.org> Sender: "Debbugs-submit" Resent-Message-ID: List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: guile-devel Cc: 28784@debbugs.gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hello, While debugging , I was surprised to see that revealed ports were not GC=E2=80=99d at all. The manual suggests othe= rwise (info "(guile) Ports and File Descriptors"): If a port=E2=80=99s revealed count is greater than zero, the file descrip= tor will not be closed when the port is garbage collected. That revealed ports are not GC=E2=80=99d makes it easy to shoot oneself in = the foot the way I did (huge memory leak). So what about the simplification below? It creates one observable difference: =E2=80=98close-port=E2=80=99 does not= hing on a revealed port, whereas until now it would close the port even if it has a non-zero revealed count. I would argue that it=E2=80=99s acceptable thou= gh. Thoughts? Ludo=E2=80=99. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=t.patch Content-Description: the patch diff --git a/libguile/fports.c b/libguile/fports.c index 94092b872..ee6ac0bf1 100644 --- a/libguile/fports.c +++ b/libguile/fports.c @@ -1,6 +1,6 @@ /* Copyright (C) 1995, 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, * 2004, 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, - * 2014, 2015 Free Software Foundation, Inc. + * 2014, 2015, 2017 Free Software Foundation, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public License @@ -467,8 +467,6 @@ fport_input_waiting (SCM port) #define SCM_REVEALED(x) (SCM_FSTREAM(x)->revealed) -static SCM revealed_ports = SCM_EOL; -static scm_i_pthread_mutex_t revealed_lock = SCM_I_PTHREAD_MUTEX_INITIALIZER; /* Find a port in the table and return its revealed count. Also used by the garbage collector. @@ -476,13 +474,7 @@ static scm_i_pthread_mutex_t revealed_lock = SCM_I_PTHREAD_MUTEX_INITIALIZER; int scm_revealed_count (SCM port) { - int ret; - - scm_i_pthread_mutex_lock (&revealed_lock); - ret = SCM_REVEALED (port); - scm_i_pthread_mutex_unlock (&revealed_lock); - - return ret; + return SCM_REVEALED (port); } SCM_DEFINE (scm_port_revealed, "port-revealed", 1, 0, 0, @@ -503,25 +495,14 @@ SCM_DEFINE (scm_set_port_revealed_x, "set-port-revealed!", 2, 0, 0, "The return value is unspecified.") #define FUNC_NAME s_scm_set_port_revealed_x { - int r, prev; + int r; port = SCM_COERCE_OUTPORT (port); SCM_VALIDATE_OPFPORT (1, port); r = scm_to_int (rcount); - - scm_i_pthread_mutex_lock (&revealed_lock); - - prev = SCM_REVEALED (port); SCM_REVEALED (port) = r; - if (r && !prev) - revealed_ports = scm_cons (port, revealed_ports); - else if (prev && !r) - revealed_ports = scm_delq_x (port, revealed_ports); - - scm_i_pthread_mutex_unlock (&revealed_lock); - return SCM_UNSPECIFIED; } #undef FUNC_NAME @@ -539,18 +520,7 @@ SCM_DEFINE (scm_adjust_port_revealed_x, "adjust-port-revealed!", 2, 0, 0, SCM_VALIDATE_OPFPORT (1, port); a = scm_to_int (addend); - if (!a) - return SCM_UNSPECIFIED; - - scm_i_pthread_mutex_lock (&revealed_lock); - SCM_REVEALED (port) += a; - if (SCM_REVEALED (port) == a) - revealed_ports = scm_cons (port, revealed_ports); - else if (!SCM_REVEALED (port)) - revealed_ports = scm_delq_x (port, revealed_ports); - - scm_i_pthread_mutex_unlock (&revealed_lock); return SCM_UNSPECIFIED; } @@ -668,6 +638,11 @@ fport_close (SCM port) { scm_t_fport *fp = SCM_FSTREAM (port); + if (SCM_REVEALED (port) > 0) + /* The port has a non-zero revealed count, so don't close the + underlying file descriptor. */ + return; + scm_run_fdes_finalizers (fp->fdes); if (close (fp->fdes) != 0) /* It's not useful to retry after EINTR, as the file descriptor is diff --git a/libguile/fports.h b/libguile/fports.h index afb8ba771..e397fcc59 100644 --- a/libguile/fports.h +++ b/libguile/fports.h @@ -3,7 +3,8 @@ #ifndef SCM_FPORTS_H #define SCM_FPORTS_H -/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2006, 2008, 2009, 2011, 2012 Free Software Foundation, Inc. +/* Copyright (C) 1995-2001, 2006, 2008, 2009, 2011, 2012, + * 2017 Free Software Foundation, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public License @@ -33,9 +34,8 @@ typedef struct scm_t_fport { /* The file descriptor. */ int fdes; - /* Revealed count; 0 indicates not revealed, > 1 revealed. Revealed - ports do not get garbage-collected. */ - int revealed; + /* Revealed count; 0 indicates not revealed, > 1 revealed. */ + unsigned int revealed; /* Set of scm_fport_option flags. */ unsigned options; } scm_t_fport; --=-=-=--