From 2acba9e5af7d898cfb5ab89c5c8c1f79050d6632 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Mon, 13 Feb 2017 00:19:42 +0000 Subject: [PATCH] ConnectionPool: avoid undefined behavior for hash iteration Perl 5.18 stable and later (commit a7b39f85d7caac) introduced a warning for restarting `each` after hash modification. While we accounted for this undefined behavior and documented it in the past, this may still cause maintenance problems in the future despite our current workarounds being sufficient. In any case, keeping idle sockets around is cheap with modern APIs, and conn_pool_size was introduced in 2.72 to avoid dropping idle connections at all; so _conn_drop_idle may never be called on a properly configured tracker. Mailing list references: <20160114024652.GA4403@dcvr.yhbt.net> --- lib/MogileFS/ConnectionPool.pm | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/lib/MogileFS/ConnectionPool.pm b/lib/MogileFS/ConnectionPool.pm index 88d0eca..7d4b00f 100644 --- a/lib/MogileFS/ConnectionPool.pm +++ b/lib/MogileFS/ConnectionPool.pm @@ -397,18 +397,11 @@ sub _conn_drop_idle { my ($self) = @_; my $idle = $self->{idle}; - # using "each" on the hash since it preserves the internal iterator - # of the hash across invocations of this sub. This should preserve - # the balance of idle connections in a big pool with many hosts. - # Thus we loop twice to ensure we scan the entire idle connection - # pool if needed - foreach (1..2) { - while (my (undef, $val) = each %$idle) { - my $conn = shift @$val or next; - - $conn->close("idle_expire") if $conn->sock; - return; - } + foreach my $val (values %$idle) { + my $conn = shift @$val or next; + + $conn->close("idle_expire") if $conn->sock; + return; } confess("BUG: unable to drop an idle connection"); -- 2.11.4.GIT