* [PATCH 1/2] Add setup and teardown for ipv6 tests @ 2013-09-09 19:03 Hleb Valoshka 2013-09-09 19:03 ` [PATCH 2/2] Return real stats instead of True Hleb Valoshka 2014-11-14 3:15 ` [PATCH 1/2] Add setup and teardown for ipv6 tests Eric Wong 0 siblings, 2 replies; 10+ messages in thread From: Hleb Valoshka @ 2013-09-09 19:03 UTC (permalink / raw) To: raindrops --- test/test_linux_ipv6.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/test_linux_ipv6.rb b/test/test_linux_ipv6.rb index ec08f28..9e8730a 100644 --- a/test/test_linux_ipv6.rb +++ b/test/test_linux_ipv6.rb @@ -12,6 +12,14 @@ class TestLinuxIPv6 < Test::Unit::TestCase TEST_ADDR = ENV["TEST_HOST6"] || "::1" + def setup + @to_close = [] + end + + def teardown + @to_close.each { |io| io.close unless io.closed? } + end + def test_tcp s = TCPServer.new(TEST_ADDR, 0) port = s.addr[1] -- 1.8.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] Return real stats instead of True 2013-09-09 19:03 [PATCH 1/2] Add setup and teardown for ipv6 tests Hleb Valoshka @ 2013-09-09 19:03 ` Hleb Valoshka 2013-09-09 19:20 ` Eric Wong 2014-11-14 3:15 ` [PATCH 1/2] Add setup and teardown for ipv6 tests Eric Wong 1 sibling, 1 reply; 10+ messages in thread From: Hleb Valoshka @ 2013-09-09 19:03 UTC (permalink / raw) To: raindrops --- ext/raindrops/linux_inet_diag.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ext/raindrops/linux_inet_diag.c b/ext/raindrops/linux_inet_diag.c index cd4a876..bae1202 100644 --- a/ext/raindrops/linux_inet_diag.c +++ b/ext/raindrops/linux_inet_diag.c @@ -620,11 +620,9 @@ static VALUE tcp_listener_stats(int argc, VALUE *argv, VALUE self) return rv; } for (i = 0; i < len; i++) { - union any_addr check; VALUE cur = rb_ary_entry(addrs, i); - parse_addr(&check, cur); - rb_hash_aset(rv, cur, Qtrue); + rb_hash_aset(rv, cur, tcp_stats(&args, cur)); } /* fall through */ } -- 1.8.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Return real stats instead of True 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 2016-02-25 9:54 ` Eric Wong 0 siblings, 2 replies; 10+ messages in thread From: Eric Wong @ 2013-09-09 19:20 UTC (permalink / raw) To: raindrops 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. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Return real stats instead of True 2013-09-09 19:20 ` Eric Wong @ 2013-09-09 19:37 ` Hleb Valoshka 2013-09-09 19:44 ` Eric Wong 2016-02-25 9:54 ` Eric Wong 1 sibling, 1 reply; 10+ messages in thread From: Hleb Valoshka @ 2013-09-09 19:37 UTC (permalink / raw) To: raindrops On 9/9/13, Eric Wong <normalperson@yhbt.net> wrote: > 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. You better reject it. I've made it because of test failure: Error: test_tcp_multi(TestLinuxIPv6) NoMethodError: undefined method `queued' for true:TrueClass test/test_linux_ipv6.rb:54:in `test_tcp_multi' 51: addrs = [ addr1, addr2 ] 52: stats = tcp_listener_stats(addrs) 53: assert_equal 2, stats.size => 54: assert_equal 0, stats[addr1].queued 55: assert_equal 0, stats[addr1].active 56: assert_equal 0, stats[addr2].queued 57: assert_equal 0, stats[addr2].active With that changes test's passed. (I didn't noticed that time that you process that Qtrues). But the real reason may be something else: I've got IPv6 address using Teredo and getnameinfo returns error with it. So it cause other errors, I need to check it better. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Return real stats instead of True 2013-09-09 19:37 ` Hleb Valoshka @ 2013-09-09 19:44 ` Eric Wong 2013-09-09 20:35 ` Hleb Valoshka 0 siblings, 1 reply; 10+ messages in thread From: Eric Wong @ 2013-09-09 19:44 UTC (permalink / raw) To: raindrops Hleb Valoshka <375gnu@gmail.com> wrote: > On 9/9/13, Eric Wong <normalperson@yhbt.net> wrote: > > 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. > > You better reject it. I've made it because of test failure: Ah, thanks. I've been forgetting to test on IPv6-enabled systems :x (I typically run custom kernels without IPv6, there were compatibility/slowdown issues (way) back in the day and it was easier to just not enable IPv6 at all. Probably moot at this point...) > But the real reason may be something else: I've got IPv6 address using > Teredo and getnameinfo returns error with it. So it cause other > errors, I need to check it better. OK. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Return real stats instead of True 2013-09-09 19:44 ` Eric Wong @ 2013-09-09 20:35 ` Hleb Valoshka 2013-09-09 22:03 ` Eric Wong 0 siblings, 1 reply; 10+ messages in thread From: Hleb Valoshka @ 2013-09-09 20:35 UTC (permalink / raw) To: raindrops On 9/9/13, Eric Wong <normalperson@yhbt.net> wrote: >> But the real reason may be something else: I've got IPv6 address using >> Teredo and getnameinfo returns error with it. So it cause other >> errors, I need to check it better. > OK. Using some debug output I've found the reason (it's not a teredo, it was disabled). Lets look into linux_inet_diag.c, if tcp_listener_stats has only 1 arg, you do rb_hash_aset(rv, addrs, tcp_stats(&args, addrs)); (or cur = rb_ary_entry(addrs, 0); rb_hash_aset(rv, cur, tcp_stats(&args, cur)); ) return rv; But if it has more agrs, you do for (i = 0; i < len; i++) { union any_addr check; VALUE cur = rb_ary_entry(addrs, i); parse_addr(&check, cur); rb_hash_aset(rv, cur, Qtrue); and then fallback to branch where arg is nil, the most interesting thing is calling of st_AND_hash(key, stats, hash) which has if (rb_hash_lookup(hash, key) == Qtrue) { VALUE v = rb_listen_stats(stats); OBJ_FREEZE(key); rb_hash_aset(hash, key, v); } The problem is `key'. It may differ from what you expect. For example, in tests you do TEST_ADDR = ENV["TEST_HOST6"] || "::1" addr1, addr2 = "[#{TEST_ADDR}]:#{port1}", "[#{TEST_ADDR}]:#{port2}" In my last case they are [::1]:42244, [::1]:46335, but values of `key' for them are: [::1%3336]:42244, [::1%3336]:46335, so rb_hash_lookup never returns Qtrue. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Return real stats instead of True 2013-09-09 20:35 ` Hleb Valoshka @ 2013-09-09 22:03 ` Eric Wong 2013-09-10 7:18 ` Hleb Valoshka 0 siblings, 1 reply; 10+ messages in thread From: Eric Wong @ 2013-09-09 22:03 UTC (permalink / raw) To: raindrops Thanks for the analysis. Can you send a fix? Otherwise I'll try to take care of it within 2 weeks (going on vacation in a few days :) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Return real stats instead of True 2013-09-09 22:03 ` Eric Wong @ 2013-09-10 7:18 ` Hleb Valoshka 0 siblings, 0 replies; 10+ messages in thread From: Hleb Valoshka @ 2013-09-10 7:18 UTC (permalink / raw) To: raindrops On 9/10/13, Eric Wong <normalperson@yhbt.net> wrote: > Thanks for the analysis. Can you send a fix? Otherwise I'll try to > take care of it within 2 weeks (going on vacation in a few days :) OK, I'll try ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] Return real stats instead of True 2013-09-09 19:20 ` Eric Wong 2013-09-09 19:37 ` Hleb Valoshka @ 2016-02-25 9:54 ` Eric Wong 1 sibling, 0 replies; 10+ messages in thread From: Eric Wong @ 2016-02-25 9:54 UTC (permalink / raw) To: Hleb Valoshka; +Cc: raindrops-public 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] Add setup and teardown for ipv6 tests 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 @ 2014-11-14 3:15 ` Eric Wong 1 sibling, 0 replies; 10+ messages in thread From: Eric Wong @ 2014-11-14 3:15 UTC (permalink / raw) To: raindrops Oops, forgot this trivial patch. Applied and pushed. New release + mailing list coming soon. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-02-25 9:54 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 2014-11-14 3:15 ` [PATCH 1/2] Add setup and teardown for ipv6 tests Eric Wong
Code repositories for project(s) associated with this public inbox https://yhbt.net/raindrops.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).