raindrops RubyGem user+dev discussion/patches/pulls/bugs/help
 help / color / Atom feed
From: Eric Wong <e@80x24.org>
To: Hleb Valoshka <375gnu@gmail.com>
Cc: raindrops-public@bogomips.org
Subject: Re: [PATCH 2/2] Return real stats instead of True
Date: Thu, 25 Feb 2016 09:54:32 +0000
Message-ID: <20160225095432.GA21370@dcvr.yhbt.net> (raw)
In-Reply-To: <20130909192020.GA18010@dcvr.yhbt.net>

Eric Wong <normalperson@yhbt.net> wrote:
> Hleb Valoshka <375gnu@gmail.com> wrote:
> 
> <no commit message body>
> 
> This deserves an explanation (and ideally, a test case).  I haven't
> looked at the code in a while and needed to think a bit about
> when the fix is relevant.

Resurrecting this from the old list 2.5 years ago:
http://bogomips.org/raindrops-public/20130909192020.GA18010@dcvr.yhbt.net/t/#u

I actually found a different reason for this problem.  Normally,
I never change the set of listeners.  But lately I've been
switching between backend HTTP servers that needed to run over TCP
(as opposed to Unix sockets) and dropping listeners occasionally
was causing "true" to show up in the results.

-----------8<------------
Subject: [PATCH] linux: tcp_listener_stats drops "true" placeholders

With invalid addresses specified which give no currently-bound
address, we must avoid leaving placeholders ('true' objects)
in our results.

Clean up some shadowing "cur" while we're at it.
---
 ext/raindrops/linux_inet_diag.c | 15 ++++++++++++---
 test/test_linux.rb              |  7 +++++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/ext/raindrops/linux_inet_diag.c b/ext/raindrops/linux_inet_diag.c
index 35bb127..58415c6 100644
--- a/ext/raindrops/linux_inet_diag.c
+++ b/ext/raindrops/linux_inet_diag.c
@@ -618,6 +618,13 @@ static VALUE tcp_stats(struct nogvl_args *args, VALUE addr)
 	return rb_listen_stats(&args->stats);
 }
 
+static int drop_placeholders(st_data_t k, st_data_t v, st_data_t ign)
+{
+	if ((VALUE)v == Qtrue)
+		return ST_DELETE;
+	return ST_CONTINUE;
+}
+
 /*
  * call-seq:
  *      Raindrops::Linux.tcp_listener_stats([addrs[, sock]]) => hash
@@ -658,10 +665,9 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self)
 	case T_ARRAY: {
 		long i;
 		long len = RARRAY_LEN(addrs);
-		VALUE cur;
 
 		if (len == 1) {
-			cur = rb_ary_entry(addrs, 0);
+			VALUE cur = rb_ary_entry(addrs, 0);
 
 			rb_hash_aset(rv, cur, tcp_stats(&args, cur));
 			return rv;
@@ -671,7 +677,7 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self)
 			VALUE cur = rb_ary_entry(addrs, i);
 
 			parse_addr(&check, cur);
-			rb_hash_aset(rv, cur, Qtrue);
+			rb_hash_aset(rv, cur, Qtrue /* placeholder */);
 		}
 		/* fall through */
 	}
@@ -689,6 +695,9 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self)
 	st_foreach(args.table, NIL_P(addrs) ? st_to_hash : st_AND_hash, rv);
 	st_free_table(args.table);
 
+	if (RHASH_SIZE(rv) > 1)
+		rb_hash_foreach(rv, drop_placeholders, Qfalse);
+
 	/* let GC deal with corner cases */
 	if (argc < 2) rb_io_close(sock);
 	return rv;
diff --git a/test/test_linux.rb b/test/test_linux.rb
index 0e79a86..bfefcc4 100644
--- a/test/test_linux.rb
+++ b/test/test_linux.rb
@@ -214,6 +214,13 @@ def test_tcp_multi
     assert_equal 0, stats[addr1].active
     assert_equal 1, stats[addr2].queued
     assert_equal 1, stats[addr2].active
+
+    # make sure we don't leave "true" placeholders in results if a
+    # listener becomes invalid (even momentarily).
+    s2.close
+    stats = tcp_listener_stats(addrs)
+    assert stats.values.all? { |x| x.instance_of?(Raindrops::ListenStats) },
+      "placeholders left: #{stats.inspect}"
   end
 
   # tries to overflow buffers
-- 
EW

  parent reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-09 19:03 [PATCH 1/2] Add setup and teardown for ipv6 tests Hleb Valoshka
2013-09-09 19:03 ` [PATCH 2/2] Return real stats instead of True Hleb Valoshka
2013-09-09 19:20   ` Eric Wong
2013-09-09 19:37     ` Hleb Valoshka
2013-09-09 19:44       ` Eric Wong
2013-09-09 20:35         ` Hleb Valoshka
2013-09-09 22:03           ` Eric Wong
2013-09-10  7:18             ` Hleb Valoshka
2016-02-25  9:54     ` Eric Wong [this message]
2014-11-14  3:15 ` [PATCH 1/2] Add setup and teardown for ipv6 tests Eric Wong

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://yhbt.net/raindrops/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160225095432.GA21370@dcvr.yhbt.net \
    --to=e@80x24.org \
    --cc=375gnu@gmail.com \
    --cc=raindrops-public@bogomips.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

raindrops RubyGem user+dev discussion/patches/pulls/bugs/help

Archives are clonable:
	git clone --mirror https://yhbt.net/raindrops-public
	git clone --mirror http://ou63pmih66umazou.onion/raindrops-public

Example config snippet for mirrors

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.lang.ruby.raindrops
	nntp://ou63pmih66umazou.onion/inbox.comp.lang.ruby.raindrops

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git