From dbc384a6ba4f719f0e24b3aab0928df551c82b25 Mon Sep 17 00:00:00 2001 From: Andy Wingo Date: Wed, 14 May 2025 14:24:12 +0200 Subject: [PATCH] Remove weak set usage in ports.c * libguile/ephemerons.h: * libguile/ephemerons.c (scm_c_ephemeron_load, scm_c_ephemeron_push): New routines. * libguile/ioext.c (scm_fdes_to_ports): Rework in terms of scm_c_port_for_each. * libguile/ports-internal.h (struct scm_t_port): Add pointer to entry on weak ports list, so that we can cancel it easily. * libguile/ports.c (release_port): Mark weak ports list entry as dead. (all_ports_needing_close, for_each_port_needing_close): Rework as ephemeron chain. (scm_c_make_port_with_encoding, close_port, scm_c_port_for_each): Adapt API. (scm_init_ports): No more weak set. --- libguile/ephemerons.c | 12 +++++++++ libguile/ephemerons.h | 4 +++ libguile/ioext.c | 32 ++++++++++++---------- libguile/ports-internal.h | 9 ++++++- libguile/ports.c | 56 +++++++++++++++++---------------------- libguile/ports.h | 5 ---- 6 files changed, 67 insertions(+), 51 deletions(-) diff --git a/libguile/ephemerons.c b/libguile/ephemerons.c index 9995d8e79..f8b7ecf54 100644 --- a/libguile/ephemerons.c +++ b/libguile/ephemerons.c @@ -126,6 +126,18 @@ scm_c_ephemeron_next (struct gc_ephemeron *e) return gc_ephemeron_chain_next (e); } +struct gc_ephemeron* +scm_c_ephemeron_load (struct gc_ephemeron **loc) +{ + return gc_ephemeron_chain_head (loc); +} + +void +scm_c_ephemeron_push (struct gc_ephemeron **loc, struct gc_ephemeron *e) +{ + gc_ephemeron_chain_push (loc, e); +} + diff --git a/libguile/ephemerons.h b/libguile/ephemerons.h index d1eb9e548..9f1fb8eb5 100644 --- a/libguile/ephemerons.h +++ b/libguile/ephemerons.h @@ -39,6 +39,10 @@ SCM_INTERNAL int scm_i_print_ephemeron (SCM exp, SCM port, SCM_INTERNAL struct gc_ephemeron* scm_to_ephemeron (SCM e); SCM_INTERNAL SCM scm_from_ephemeron (struct gc_ephemeron *e); +SCM_INTERNAL struct gc_ephemeron* scm_c_ephemeron_load (struct gc_ephemeron **loc); +SCM_INTERNAL void scm_c_ephemeron_push (struct gc_ephemeron **loc, + struct gc_ephemeron *e); + SCM_INTERNAL struct scm_ephemeron_table* scm_c_make_ephemeron_table (size_t count); SCM_INTERNAL size_t scm_c_ephemeron_table_length (struct scm_ephemeron_table *et); SCM_INTERNAL struct gc_ephemeron* diff --git a/libguile/ioext.c b/libguile/ioext.c index 8f5691184..2bae6875d 100644 --- a/libguile/ioext.c +++ b/libguile/ioext.c @@ -1,4 +1,4 @@ -/* Copyright 1995-2001,2003,2006,2011,2014,2018 +/* Copyright 1995-2001,2003,2006,2011,2014,2018,2025 Free Software Foundation, Inc. This file is part of Guile. @@ -282,15 +282,19 @@ SCM_DEFINE (scm_primitive_move_to_fdes, "primitive-move->fdes", 2, 0, 0, } #undef FUNC_NAME -static SCM -get_matching_port (void *closure, SCM port, SCM result) +struct scm_fdes_to_ports_data { - int fd = * (int *) closure; - - if (SCM_OPFPORTP (port) && SCM_FSTREAM (port)->fdes == fd) - result = scm_cons (port, result); + SCM ports; + int fd; +}; - return result; +static void +collect_matching_ports (void *closure, SCM port) +{ + struct scm_fdes_to_ports_data *data = closure; + + if (SCM_OPFPORTP (port) && SCM_FSTREAM (port)->fdes == data->fd) + data->ports = scm_cons (port, data->ports); } /* Return a list of ports using a given file descriptor. */ @@ -301,13 +305,13 @@ SCM_DEFINE (scm_fdes_to_ports, "fdes->ports", 1, 0, 0, "counts.") #define FUNC_NAME s_scm_fdes_to_ports { - SCM result = SCM_EOL; - int int_fd = scm_to_int (fd); + struct scm_fdes_to_ports_data data; + data.ports = SCM_EOL; + data.fd = scm_to_int (fd); - result = scm_c_weak_set_fold (get_matching_port, - (void*) &int_fd, result, - scm_i_port_weak_set); - return result; + scm_c_port_for_each (collect_matching_ports, &data); + + return data.ports; } #undef FUNC_NAME diff --git a/libguile/ports-internal.h b/libguile/ports-internal.h index 4e0a72f95..8433c5903 100644 --- a/libguile/ports-internal.h +++ b/libguile/ports-internal.h @@ -1,6 +1,6 @@ /* ports-internal.h - internal-only declarations for ports. - Copyright 2013,2018 + Copyright 2013,2018,2025 Free Software Foundation, Inc. This file is part of Guile. @@ -27,6 +27,7 @@ #include #include "libguile/bytevectors.h" +#include "libguile/ephemerons.h" #include "libguile/list.h" #include "libguile/pairs.h" #include "libguile/ports.h" @@ -361,6 +362,12 @@ struct scm_t_port iconv_t input_cd; /* with iconv_lock */ iconv_t output_cd; /* with iconv_lock */ + /* If the port needs to be closed when it becomes unreachable, it is + put on a global ephemeron chain, so that we can explicitly flush + all ports. If the port is later closed manually, we can mark that + chain link as dead, and it will be collected on the next GC. */ + struct gc_ephemeron *close_handle; + /* Port properties. */ SCM alist; }; diff --git a/libguile/ports.c b/libguile/ports.c index e979edada..8ddaf06c1 100644 --- a/libguile/ports.c +++ b/libguile/ports.c @@ -159,6 +159,13 @@ release_port (SCM port) if (cur > 1) return; + struct gc_ephemeron *e = pt->close_handle; + if (e) + { + pt->close_handle = NULL; + scm_c_ephemeron_mark_dead_x (e); + } + /* FIXME: `catch' around the close call? It could throw an exception, and in that case we'd leak the iconv descriptors, if any. */ if (SCM_PORT_TYPE (port)->close) @@ -701,11 +708,24 @@ SCM_DEFINE (scm_port_mode, "port-mode", 1, 0, 0, -/* The port table --- a weak set of all ports. +/* The port list --- a weak list of all ports. We need a global registry of ports to flush them all at exit, and to get all the ports matching a file descriptor. */ -SCM scm_i_port_weak_set; +static struct gc_ephemeron *all_ports_needing_close = NULL; + +static void +for_each_port_needing_close (void (*proc)(void *data, SCM p), void *data) +{ + for (struct gc_ephemeron *e = scm_c_ephemeron_load (&all_ports_needing_close); + e; + e = scm_c_ephemeron_next (e)) + { + SCM port = scm_c_ephemeron_key (e); + if (!scm_is_false (port)) + proc (data, port); + } +} @@ -798,7 +818,8 @@ scm_c_make_port_with_encoding (scm_t_port_type *ptob, unsigned long mode_bits, if (SCM_PORT_TYPE (ret)->flags & SCM_PORT_TYPE_NEEDS_CLOSE_ON_GC) { scm_i_add_port_finalizer (SCM_I_CURRENT_THREAD, ret); - scm_weak_set_add_x (scm_i_port_weak_set, ret); + pt->close_handle = scm_c_make_ephemeron (ret, SCM_BOOL_T); + scm_c_ephemeron_push (&all_ports_needing_close, pt->close_handle); } initialize_port_buffers (ret); @@ -901,9 +922,6 @@ close_port (SCM port, int explicit) SCM_CLR_PORT_OPEN_FLAG (port); - if (SCM_PORT_TYPE (port)->flags & SCM_PORT_TYPE_NEEDS_CLOSE_ON_GC) - scm_weak_set_remove_x (scm_i_port_weak_set, port); - release_port (port); return SCM_BOOL_T; @@ -4059,32 +4077,10 @@ scm_port_print (SCM exp, SCM port, scm_print_state *pstate SCM_UNUSED) /* Iterating over all ports. */ -struct for_each_data -{ - void (*proc) (void *data, SCM p); - void *data; -}; - -static SCM -for_each_trampoline (void *data, SCM port, SCM result) -{ - struct for_each_data *d = data; - - d->proc (d->data, port); - - return result; -} - void scm_c_port_for_each (void (*proc)(void *data, SCM p), void *data) { - struct for_each_data d; - - d.proc = proc; - d.data = data; - - scm_c_weak_set_fold (for_each_trampoline, &d, SCM_EOL, - scm_i_port_weak_set); + for_each_port_needing_close (proc, data); } static void @@ -4230,8 +4226,6 @@ scm_init_ports (void) scm_void_port_type = scm_make_port_type ("void", void_port_read, void_port_write); - scm_i_port_weak_set = scm_c_make_weak_set (31); - cur_inport_fluid = scm_make_fluid (); cur_outport_fluid = scm_make_fluid (); cur_errport_fluid = scm_make_fluid (); diff --git a/libguile/ports.h b/libguile/ports.h index 055e5b6b9..362236310 100644 --- a/libguile/ports.h +++ b/libguile/ports.h @@ -27,11 +27,6 @@ #include "libguile/print.h" #include "libguile/strings.h" - - -SCM_INTERNAL SCM scm_i_port_weak_set; - - #define SCM_EOF_OBJECT_P(x) (scm_is_eq ((x), SCM_EOF_VAL))