From 57ceb980e3c55306f3f70f92604703abf132cf27 Mon Sep 17 00:00:00 2001 From: Mark Wooding Date: Sat, 9 Jun 2018 17:28:14 +0100 Subject: [PATCH] fwd: Improve `source' and `target' lifecycle management. * Add reference counts and functions for handling them to `source' and `target'. * Use these instead of poking the `destroy' methods directly. * Introduce a new `source' method, `shutdown', to disentangle the source from the machinery, and use this in `source_killall'. This fixes a bug in socket sources with a connection limit: if an endpoint closes during graceful shutdown, then the endpoint tries to get the dead source to resume listening, and things go south quickly. --- Makefile.am | 1 + exec.c | 15 ++++++--------- file.c | 15 ++++++--------- fwd.c | 1 + fwd.h | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ socket.c | 30 +++++++++++++++++++++--------- source.c | 39 ++++++++++++++++++++++++++++++++++++++- target.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 194 insertions(+), 28 deletions(-) create mode 100644 target.c diff --git a/Makefile.am b/Makefile.am index 055a217..c6bf142 100644 --- a/Makefile.am +++ b/Makefile.am @@ -68,6 +68,7 @@ fwd_SOURCES += fwd.c fwd.h fwd_SOURCES += chan.c fwd_SOURCES += endpt.c fwd_SOURCES += source.c +fwd_SOURCES += target.c fwd_SOURCES += conf.c fwd_SOURCES += scan.c diff --git a/exec.c b/exec.c index d41a31d..fc8b679 100644 --- a/exec.c +++ b/exec.c @@ -1004,6 +1004,8 @@ static source *xsource_read(scanner *sc) return (0); xs = CREATE(xsource); xs->s.ops = &xsource_ops; + xs->s.ref = 1; + xs->s.f = 0; xs->s.desc = 0; exec_read(&xs->x, sc); return (&xs->s); @@ -1032,18 +1034,12 @@ static void xsource_attach(source *s, scanner *sc, target *t) /* --- Create the endpoints --- */ if ((ee = t->ops->create(t, xs->s.desc)) == 0) - goto tidy; + return; if ((e = exec_endpt(&xs->x, xs->s.desc)) == 0) { ee->ops->close(ee); - goto tidy; + return; } endpt_join(e, ee, xs->s.desc); - - /* --- Dispose of source and target --- */ - -tidy: - t->ops->destroy(t); - xsource_destroy(&xs->s); } /* --- @destroy@ --- */ @@ -1060,7 +1056,7 @@ static void xsource_destroy(source *s) source_ops xsource_ops = { "exec", - xsource_option, xsource_read, xsource_attach, xsource_destroy + xsource_option, xsource_read, xsource_attach, 0, xsource_destroy }; /*----- Exec target description -------------------------------------------*/ @@ -1084,6 +1080,7 @@ static target *xtarget_read(scanner *sc) return (0); xt = CREATE(xtarget); xt->t.ops = &xtarget_ops; + xt->t.ref = 1; exec_read(&xt->x, sc); exec_desc(&xt->x, &d); xt->t.desc = xstrdup(d.buf); diff --git a/file.c b/file.c index 8b4f533..5da247c 100644 --- a/file.c +++ b/file.c @@ -457,6 +457,8 @@ static source *fsource_read(scanner *sc) return (0); fs = CREATE(fsource); fs->s.ops = &fsource_ops; + fs->s.ref = 1; + fs->s.f = 0; fs->s.desc = 0; file_read(&fs->f, sc); return (&fs->s); @@ -485,18 +487,12 @@ static void fsource_attach(source *s, scanner *sc, target *t) /* --- And then create the endpoints --- */ if ((ee = t->ops->create(t, fs->s.desc)) == 0) - goto tidy; + return; if ((e = file_endpt(&fs->f, fs->s.desc)) == 0) { ee->ops->close(ee); - goto tidy; + return; } endpt_join(e, ee, fs->s.desc); - - /* --- Dispose of the source and target now --- */ - -tidy: - t->ops->destroy(t); - fsource_destroy(&fs->s); } /* --- @destroy@ --- */ @@ -513,7 +509,7 @@ static void fsource_destroy(source *s) source_ops fsource_ops = { "file", - fsource_option, fsource_read, fsource_attach, fsource_destroy + fsource_option, fsource_read, fsource_attach, 0, fsource_destroy }; /*----- File target description -------------------------------------------*/ @@ -537,6 +533,7 @@ static target *ftarget_read(scanner *sc) return (0); ft = CREATE(ftarget); ft->t.ops = &ftarget_ops; + ft->t.ref = 1; file_read(&ft->f, sc); file_desc(&ft->f, &d); ft->t.desc = xstrdup(d.buf); diff --git a/fwd.c b/fwd.c index 0455bc4..9d170e1 100644 --- a/fwd.c +++ b/fwd.c @@ -152,6 +152,7 @@ void parse(scanner *sc) s->ops->attach(s, sc, t); if (t->ops->confirm) t->ops->confirm(t); + source_dec(s); target_dec(t); } /* --- Include configuration from a file --- * diff --git a/fwd.h b/fwd.h index f075c53..e295c8b 100644 --- a/fwd.h +++ b/fwd.h @@ -899,6 +899,7 @@ typedef struct endpt_ops { typedef struct target { struct target_ops *ops; + unsigned ref; char *desc; } target; @@ -973,9 +974,13 @@ typedef struct target_ops { typedef struct source { struct source *next, *prev; struct source_ops *ops; + unsigned ref; + unsigned f; +#define SF_ACTIVE 1u char *desc; } source; + /* --- Forwarding source operations --- */ typedef struct source_ops { @@ -1019,6 +1024,18 @@ typedef struct source_ops { void (*attach)(source */*s*/, scanner */*sc*/, target */*t*/); + /* --- @shutdown@ --- * + * + * Arguments: @source *s@ = pointer to source + * + * Returns: --- + * + * Use: Deactivates the source so that it won't produce any more + * endpoints. + */ + + void (*shutdown)(source */*s*/); + /* --- @destroy@ --- * * * Arguments: @source *s@ = pointer to source @@ -1078,6 +1095,28 @@ extern void endpt_killall(void); extern void endpt_join(endpt */*a*/, endpt */*b*/, const char */*desc*/); +/* --- @target_inc@ --- * + * + * Arguments: @target *t@ = pointer to a source + * + * Returns: --- + * + * Use: Increments a target's refcount. + */ + +extern void target_inc(target */*t*/); + +/* --- @target_dec@ --- * + * + * Arguments: @target *t@ = pointer to a target + * + * Returns: --- + * + * Use: Decrements a target's refcount, destroying it if necessary. + */ + +extern void target_dec(target */*t*/); + /* --- @source_add@ --- * * * Arguments: @source *s@ = pointer to a source @@ -1102,6 +1141,28 @@ extern void source_add(source */*s*/); extern void source_remove(source */*s*/); +/* --- @source_inc@ --- * + * + * Arguments: @source *s@ = pointer to a source + * + * Returns: --- + * + * Use: Increments a source's refcount. + */ + +extern void source_inc(source */*s*/); + +/* --- @source_dec@ --- * + * + * Arguments: @source *s@ = pointer to a source + * + * Returns: --- + * + * Use: Decrements a source's refcount, destroying it if necessary. + */ + +extern void source_dec(source */*s*/); + /* --- @source_killall@ --- * * * Arguments: --- diff --git a/socket.c b/socket.c index a1a9a55..36e58ea 100644 --- a/socket.c +++ b/socket.c @@ -170,7 +170,7 @@ static void ssept_close(endpt *e) { ssept *ee = (ssept *)e; - if (ee->s->o.opt == SOCKOPT_LIMIT) { + if ((ee->s->s.f&SF_ACTIVE) && ee->s->o.opt == SOCKOPT_LIMIT) { ee->s->o.conn++; if (ee->s->o.conn == 1) ss_listen(ee->s); @@ -351,6 +351,8 @@ static source *ssource_read(scanner *sc) (void)(conf_prefix(sc, "socket") || conf_prefix(sc, "sk")); ss = CREATE(ssource); ss->s.ops = &ssource_ops; + ss->s.ref = 1; + ss->s.f = 0; ss->s.desc = 0; ss->t = 0; ss->a = getaddr(sc, ADDR_SRC); @@ -447,7 +449,7 @@ static void ss_accept(int fd, unsigned mode, void *p) close(ss->r.fd); if (ss->a->ops->unbind) ss->a->ops->unbind(ss->a); - ssource_destroy(&ss->s); + source_dec(&ss->s); acceptp = 0; break; } @@ -511,7 +513,7 @@ fail_1: close(fd); fail_0: ss->o.conn = 0; - ssource_destroy(&ss->s); + source_dec(&ss->s); } /* --- @attach@ --- */ @@ -522,7 +524,7 @@ static void ssource_attach(source *s, scanner *sc, target *t) int fd; int opt = 1; - ss->t = t; + ss->t = t; target_inc(t); /* --- Initialize the description string --- */ @@ -563,9 +565,9 @@ static void ssource_attach(source *s, scanner *sc, target *t) fw_inc(); } -/* --- @destroy@ --- */ +/* --- @shutdown@ --- */ -static void ssource_destroy(source *s) +static void ssource_shutdown(source *s) { ssource *ss = (ssource *)s; @@ -579,10 +581,19 @@ static void ssource_destroy(source *s) ss->a->ops->freesrcopts(ss->ao); else DESTROY(ss->ao); - xfree(ss->s.desc); ss->a->ops->destroy(ss->a); - ss->t->ops->destroy(ss->t); source_remove(&ss->s); + target_dec(ss->t); + fw_dec(); +} + +/* --- @destroy@ --- */ + +static void ssource_destroy(source *s) +{ + ssource *ss = (ssource *)s; + + xfree(ss->s.desc); DESTROY(ss); fw_dec(); } @@ -591,7 +602,7 @@ static void ssource_destroy(source *s) source_ops ssource_ops = { "socket", - ssource_option, ssource_read, ssource_attach, ssource_destroy + ssource_option, ssource_read, ssource_attach, ssource_shutdown, ssource_destroy }; /*----- Target definition -------------------------------------------------*/ @@ -627,6 +638,7 @@ static target *starget_read(scanner *sc) (void)(conf_prefix(sc, "socket") || conf_prefix(sc, "sk")); st = CREATE(starget); st->t.ops = &starget_ops; + st->t.ref = 1; st->a = getaddr(sc, ADDR_DEST); if (st->a->ops->inittargopts) st->ao = st->a->ops->inittargopts(); diff --git a/source.c b/source.c index 96b7f81..f515ae2 100644 --- a/source.c +++ b/source.c @@ -45,11 +45,15 @@ static source *sources = 0; void source_add(source *s) { + assert(s->ops->shutdown); + assert(!(s->f&SF_ACTIVE)); s->next = sources; s->prev = 0; if (sources) sources->prev = s; sources = s; + s->f |= SF_ACTIVE; + source_inc(s); } /* --- @source_remove@ --- * @@ -63,12 +67,45 @@ void source_add(source *s) void source_remove(source *s) { + assert(s->f&SF_ACTIVE); if (s->next) s->next->prev = s->prev; if (s->prev) s->prev->next = s->next; else sources = s->next; + s->f &= ~SF_ACTIVE; + source_dec(s); +} + +/* --- @source_inc@ --- * + * + * Arguments: @source *s@ = pointer to a source + * + * Returns: --- + * + * Use: Increments a source's refcount. + */ + +void source_inc(source *s) { s->ref++; } + +/* --- @source_dec@ --- * + * + * Arguments: @source *s@ = pointer to a source + * + * Returns: --- + * + * Use: Decrements a source's refcount, destroying it if necessary. + */ + +void source_dec(source *s) +{ + assert(s->ref > 0); + s->ref--; + if (!s->ref) { + if (s->f&SF_ACTIVE) s->ops->shutdown(s); + s->ops->destroy(s); + } } /* --- @source_killall@ --- * @@ -86,7 +123,7 @@ void source_killall(void) while (s) { source *ss = s; s = s->next; - ss->ops->destroy(ss); + ss->ops->shutdown(ss); } sources = 0; } diff --git a/target.c b/target.c new file mode 100644 index 0000000..5590761 --- /dev/null +++ b/target.c @@ -0,0 +1,60 @@ +/* -*-c-*- + * + * Common functionality for targets + * + * (c) 2018 Straylight/Edgeware + */ + +/*----- Licensing notice --------------------------------------------------* + * + * This file is part of the `fwd' port forwarder. + * + * `fwd' is free software: you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free + * Software Foundation; either version 2 of the License, or (at your + * option) any later version. + * + * `fwd' is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * for more details. + * + * You should have received a copy of the GNU General Public License + * along with `fwd'. If not, write to the Free Software Foundation, + * Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. + */ + +/*----- Header files ------------------------------------------------------*/ + +#include "fwd.h" + +/*----- Main code ---------------------------------------------------------*/ + +/* --- @target_inc@ --- * + * + * Arguments: @target *t@ = pointer to a source + * + * Returns: --- + * + * Use: Increments a target's refcount. + */ + +void target_inc(target *t) { t->ref++; } + +/* --- @target_dec@ --- * + * + * Arguments: @target *t@ = pointer to a target + * + * Returns: --- + * + * Use: Decrements a target's refcount, destroying it if necessary. + */ + +void target_dec(target *t) +{ + assert(t->ref > 0); + t->ref--; + if (!t->ref) t->ops->destroy(t); +} + +/*----- That's all, folks -------------------------------------------------*/ -- 2.11.0