From: Maria Matejka Date: Sat, 24 May 2025 12:16:44 +0000 (+0200) Subject: Table: Do not attempt to prune an empty table X-Git-Tag: v3.1.2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=565a4c4fc9d7c5f91eb46e9fcda8797ec152148c;p=thirdparty%2Fbird.git Table: Do not attempt to prune an empty table Table pruning is requested from multiple places, including a timer. Sometimes this may happen in a race condition with table shutdown, and therefore we explicitly refuse to schedule pruning if the table is empty. Also added a full-blown check that a shutting-down table is indeed empty, and several more asserts to catch imminent crashes before they happen in hard-to-debug places. This fixes #254. --- diff --git a/nest/rt-export.c b/nest/rt-export.c index 7df334a87..98473b155 100644 --- a/nest/rt-export.c +++ b/nest/rt-export.c @@ -455,6 +455,7 @@ void rt_exporter_init(struct rt_exporter *e, struct settle_config *scf) { rtex_trace(e, D_STATES, "Exporter init"); + ASSERT_DIE(e->journal.domain); e->journal.cleanup_done = rt_exporter_cleanup_done; lfjour_init(&e->journal, scf); ASSERT_DIE(e->feed_net); @@ -547,10 +548,7 @@ rt_exporter_shutdown(struct rt_exporter *e, void (*stopped)(struct rt_exporter * rtex_trace(e, D_STATES, "Exporter shutdown"); /* Last lock check before dropping the domain reference */ - if (e->journal.domain) - ASSERT_DIE(DG_IS_LOCKED(e->journal.domain)); - - e->journal.domain = NULL; + ASSERT_DIE(DG_IS_LOCKED(e->journal.domain)); /* We have to tell every receiver to stop */ bool done = 1; @@ -576,13 +574,22 @@ rt_exporter_shutdown(struct rt_exporter *e, void (*stopped)(struct rt_exporter * /* Wait for feeders to finish */ synchronize_rcu(); - /* The rest is done via the cleanup routine */ - lfjour_do_cleanup_now(&e->journal); - if (done) { + /* Inhibit locking in cleanup */ + e->journal.domain = NULL; + + /* The rest is done via the cleanup routine */ + lfjour_do_cleanup_now(&e->journal); + + /* Invalidate the cleanup event */ + e->journal.cleanup_event.hook = NULL; ev_postpone(&e->journal.cleanup_event); + + /* No announcement timer either */ settle_cancel(&e->journal.announce_timer); + + /* Done! */ CALL(stopped, e); } else diff --git a/nest/rt-table.c b/nest/rt-table.c index 888753374..cc79be193 100644 --- a/nest/rt-table.c +++ b/nest/rt-table.c @@ -2995,6 +2995,10 @@ rt_schedule_nhu(struct rtable_private *tab) void rt_schedule_prune(struct rtable_private *tab) { + /* The table is empty if there are no imports */ + if (EMPTY_LIST(tab->imports)) + return; + /* state change 0->1, 2->3 */ tab->prune_state |= 1; if (!tab->reconf_end) @@ -4695,6 +4699,15 @@ rt_shutdown(void *tab_) { rtable *t = tab_; RT_LOCK(t, tab); + ASSERT_DIE(birdloop_inside(tab->loop)); + + /* Check that the table is indeed pruned */ + tab->prune_state = 0; + ASSERT_DIE(EMPTY_LIST(tab->imports)); + u32 bs = atomic_load_explicit(&tab->routes_block_size, memory_order_relaxed); + net *routes = atomic_load_explicit(&tab->routes, memory_order_relaxed); + for (u32 i = 0; i < bs; i++) + ASSERT_DIE(atomic_load_explicit(&routes[i].routes, memory_order_relaxed) == NULL); if (tab->export_digest) { diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c index 5843183e6..77b8c806e 100644 --- a/proto/bgp/attrs.c +++ b/proto/bgp/attrs.c @@ -2141,6 +2141,7 @@ bgp_init_pending_tx(struct bgp_channel *c) bpp->exporter = (struct rt_exporter) { .journal = { .loop = c->c.proto->loop, + .domain = dom.rtable, .item_size = sizeof(struct rt_export_item), .item_done = bgp_out_item_done, },