[PATCH 0/7] PSGI-related updates

[PATCH 0/7] PSGI-related updates

[Eric Wong]

I'm using it in "production" on an httpd with live traffic
and have found two confirmed leakks so far.

Eric Wong (7):
  don't set root_locating until BOOT
  examples/mwrap.psgi: explain why we delete LD_PRELOAD
  psgi: fix 404 return
  examples/mwrap.psgi: disable deflater
  psgi: add total bytes + reset
  psgi: sort "location" column lexically
  psgi: show address column

 Mwrap.xs                | 24 +++++++++++----
 examples/mwrap.psgi     | 10 +-----
 lib/Devel/Mwrap/PSGI.pm | 67 +++++++++++++++++++++++++----------------
 3 files changed, 60 insertions(+), 41 deletions(-)

[PATCH 1/7] don't set root_locating until BOOT

[Eric Wong]

We don't know if a thread is Perl or not until it boots.
---
 Mwrap.xs | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Mwrap.xs b/Mwrap.xs
index e9b8b3f..b93efd6 100644
--- a/Mwrap.xs
+++ b/Mwrap.xs
@@ -95,9 +95,7 @@ lfht_new(void)
 __attribute__((constructor)) static void resolve_malloc(void)
 {
 	int err;
-#ifndef PERL_IMPLICIT_CONTEXT
-	root_locating = &locating;
-#endif
+
 	++locating;
 
 #ifdef __FreeBSD__

[PATCH 2/7] examples/mwrap.psgi: explain why we delete LD_PRELOAD

[Eric Wong]

This example is documentation, after all.
---
 examples/mwrap.psgi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/mwrap.psgi b/examples/mwrap.psgi
index be814fe..41edd76 100644
--- a/examples/mwrap.psgi
+++ b/examples/mwrap.psgi
@@ -6,7 +6,7 @@ use v5.12;
 use Devel::Mwrap::PSGI;
 use Plack::Builder;
 my $mw = Devel::Mwrap::PSGI->new;
-delete $ENV{LD_PRELOAD};
+delete $ENV{LD_PRELOAD}; # don't impose in subprocesses
 
 builder {
 	eval {

[PATCH 3/7] psgi: fix 404 return

[Eric Wong]

---
 lib/Devel/Mwrap/PSGI.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/Devel/Mwrap/PSGI.pm b/lib/Devel/Mwrap/PSGI.pm
index 90b4354..f47c9b0 100644
--- a/lib/Devel/Mwrap/PSGI.pm
+++ b/lib/Devel/Mwrap/PSGI.pm
@@ -164,7 +164,7 @@ sub call { # PSGI entry point
 		$ret = [ 200, [ qw(Content-Type text/html
 				Content-Length), length($root) ], [ $root ] ]
 	} else {
-		r404();
+		$ret = r404();
 	}
 	Devel::Mwrap::quiet(0);
 	$ret;

[PATCH 4/7] examples/mwrap.psgi: disable deflater

[Eric Wong]

It probably doesn't make sense to enable in this example
since Mwrap's PSGI output is only meant for localhost users.
---
 examples/mwrap.psgi | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/examples/mwrap.psgi b/examples/mwrap.psgi
index 41edd76..8eb66b0 100644
--- a/examples/mwrap.psgi
+++ b/examples/mwrap.psgi
@@ -9,14 +9,6 @@ my $mw = Devel::Mwrap::PSGI->new;
 delete $ENV{LD_PRELOAD}; # don't impose in subprocesses
 
 builder {
-	eval {
-		enable 'Deflater',
-			content_type => [ qw(
-				text/html
-				text/plain
-				application/atom+xml
-				)]
-	}; # Plack::Middleware::Deflater may not be installed
 	enable 'Head';
 	sub { $mw->call($_[0]) };
 }

[PATCH 5/7] psgi: add total bytes + reset

[Eric Wong]

These will probably be more useful, we'll drop total bytes
allocated, for now.
---
 Mwrap.xs                | 15 ++++++++++++
 lib/Devel/Mwrap/PSGI.pm | 54 ++++++++++++++++++++++++-----------------
 2 files changed, 47 insertions(+), 22 deletions(-)

diff --git a/Mwrap.xs b/Mwrap.xs
index b93efd6..43d1ed6 100644
--- a/Mwrap.xs
+++ b/Mwrap.xs
@@ -202,6 +202,7 @@ static char *int2str(unsigned num, char *dst, size_t * size)
 struct src_loc {
 	pthread_mutex_t *mtx;
 	size_t total;
+	size_t freed_bytes;
 	size_t allocations;
 	size_t frees;
 	size_t age_total; /* (age_total / frees) => mean age at free */
@@ -402,6 +403,7 @@ void free(void *p)
 			size_t current_bytes = uatomic_read(&total_bytes_inc);
 			size_t age = current_bytes - h->as.live.gen;
 			uatomic_add(&total_bytes_dec, h->size);
+			uatomic_add(&l->freed_bytes, h->size);
 			uatomic_set(&h->size, 0);
 			uatomic_add(&l->frees, 1);
 			uatomic_add(&l->age_total, age);
@@ -889,6 +891,7 @@ CODE:
 		uatomic_set(&l->total, 0);
 		uatomic_set(&l->allocations, 0);
 		uatomic_set(&l->frees, 0);
+		uatomic_set(&l->freed_bytes, 0);
 		uatomic_set(&l->age_total, 0);
 		uatomic_set(&l->max_lifespan, 0);
 	}
@@ -960,6 +963,18 @@ OUTPUT:
 CLEANUP:
 	--locating;
 
+size_t
+src_loc_freed_bytes(self)
+	Devel::Mwrap::SrcLoc self
+PREINIT:
+CODE:
+	++locating;
+	RETVAL = uatomic_read(&self->freed_bytes);
+OUTPUT:
+	RETVAL
+CLEANUP:
+	--locating;
+
 size_t
 src_loc_allocations(self)
 	Devel::Mwrap::SrcLoc self
diff --git a/lib/Devel/Mwrap/PSGI.pm b/lib/Devel/Mwrap/PSGI.pm
index f47c9b0..5bb7c36 100644
--- a/lib/Devel/Mwrap/PSGI.pm
+++ b/lib/Devel/Mwrap/PSGI.pm
@@ -39,14 +39,16 @@ sub uri_unescape {
 	$str;
 }
 
-my @FIELDS = qw(total allocations frees live mean_life max_life location);
+my @FIELDS = qw(bytes allocations frees live
+		mean_life max_life location);
 my $HDR = '<tr><th>' . join('</th><th>', @FIELDS) . '</th></tr>';
 
 sub accumulate_i { # callback for Devel::Mwrap::each
 	my ($all, $src_loc) = @_;
 	my $alloc = $src_loc->allocations;
 	my $frees = $src_loc->frees;
-	push @$all, [ $src_loc->total, $alloc, $frees, $alloc - $frees,
+	push @$all, [ $src_loc->total - $src_loc->freed_bytes,
+			$alloc, $frees, $alloc - $frees,
 			$src_loc->mean_lifespan, $src_loc->max_lifespan,
 			$src_loc->name ];
 }
@@ -65,9 +67,10 @@ sub fh_response {
 }
 
 sub each_gt {
-	my ($env, $min, $sort) = @_;
+	my ($env, $min) = @_;
+	my ($sort) = ($env->{QUERY_STRING} =~ /\bsort=(\w+)\b/a);
 	open my $fh, '+>', undef or die "open: $!";
-	$sort //= 'total';
+	$sort //= 'bytes';
 	my $sn = $env->{SCRIPT_NAME};
 	my $t = "Devel::Mwrap::each($min)";
 	my $all = [];
@@ -93,12 +96,12 @@ sub each_gt {
 	my $age = Devel::Mwrap::current_age();
 	my $live = $age - Devel::Mwrap::total_bytes_freed();
 	print $fh <<EOM;
-<html><head><title>$t</title></head><body><h1>$t</h1>
-<h2>Current age: $age (live: $live)</h2>
+<html><head><title>$t</title></head><body><p>$t
+<p>Current age: $age (live: $live)
 <table><tr><th>@f</th></tr>
 EOM
 	while (my $cols = shift @all) {
-		# cols: [ total, allocations, frees, mean_lifespan,
+		# cols: [ bytes , allocations, frees, mean_lifespan,
 		#   max_lifespan, name ]
 		my $loc_name = pop @$cols;
 		$cols->[4] = sprintf('%0.3f', $cols->[4]); # mean_life
@@ -125,8 +128,8 @@ sub each_at {
 	my $age = Devel::Mwrap::current_age();
 	my $live = $age - Devel::Mwrap::total_bytes_freed();
 	print $fh <<EOM;
-<html><head><title>$t</title></head><body><h1>live allocations at $t</h1>
-<h2>Current age: $age (live: $live)</h2>\n<table>
+<html><head><title>$t</title></head><body><p>live allocations at $t
+<p>Current age: $age (live: $live)\n<table>
 <tr><th>size</th><th>generation</th></tr>
 EOM
 	$src_loc->each(0, \&each_at_i, $fh);
@@ -151,21 +154,28 @@ EOM
 sub call { # PSGI entry point
 	Devel::Mwrap::quiet(1);
 	my (undef, $env) = @_;
-	my $path_info = $env->{PATH_INFO};
 	my $ret;
-	if ($path_info =~ m!\A/each/([0-9]+)\z!) {
-		my $min = $1 + 0;
-		my ($sort) = ($env->{QUERY_STRING} =~ /\bsort=(\w+)\b/a);
-		$ret = each_gt($env, $min, $sort);
-	} elsif ($path_info =~ m!\A/at/(.*)\z!) {
-		my $src_loc = Devel::Mwrap::get(uri_unescape($1));
-		$ret = $src_loc ? each_at($env, $src_loc) : r404();
-	} elsif ($path_info eq '/') {
-		$ret = [ 200, [ qw(Content-Type text/html
-				Content-Length), length($root) ], [ $root ] ]
-	} else {
-		$ret = r404();
+	my $path_info = $env->{PATH_INFO};
+	if ($env->{REQUEST_METHOD} eq 'GET') {
+		if ($path_info =~ m!\A/each/([0-9]+)\z!) {
+			$ret = each_gt($env, $1 + 0);
+		} elsif ($path_info =~ m!\A/at/(.*)\z!) {
+			my $src_loc = Devel::Mwrap::get(uri_unescape($1));
+			$ret = $src_loc ? each_at($env, $src_loc) : r404();
+		} elsif ($path_info eq '/') {
+			$ret = [ 200, [ qw(Content-Type text/html
+					Content-Length), length($root) ],
+				[ $root ] ]
+		}
+	} elsif ($env->{REQUEST_METHOD} eq 'POST') {
+		if ($path_info eq '/reset') {
+			Devel::Mwrap::reset();
+			$ret = [ 200, [ qw(Content-Type text/html
+					Content-Length 5) ],
+				[ "done\n" ] ];
+		}
 	}
+	$ret //= r404();
 	Devel::Mwrap::quiet(0);
 	$ret;
 }

[PATCH 6/7] psgi: sort "location" column lexically

[Eric Wong]

It's probably not worth sorting line numbers for
location.
---
 lib/Devel/Mwrap/PSGI.pm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/Devel/Mwrap/PSGI.pm b/lib/Devel/Mwrap/PSGI.pm
index 5bb7c36..52dbebe 100644
--- a/lib/Devel/Mwrap/PSGI.pm
+++ b/lib/Devel/Mwrap/PSGI.pm
@@ -92,7 +92,11 @@ sub each_gt {
 	} @f));
 	my @all;
 	Devel::Mwrap::each($min, \&accumulate_i, \@all);
-	@all = sort { $b->[$sc] <=> $a->[$sc] } @all;
+	if ($sc eq $#FIELDS) { # locations are sorted alphabetically
+		@all = sort { $a->[$sc] cmp $b->[$sc] } @all;
+	} else { # everything else is numeric
+		@all = sort { $b->[$sc] <=> $a->[$sc] } @all;
+	}
 	my $age = Devel::Mwrap::current_age();
 	my $live = $age - Devel::Mwrap::total_bytes_freed();
 	print $fh <<EOM;

[PATCH 7/7] psgi: show address column

[Eric Wong]

These addresses are useful for plugging into gdb while
gdb is hooked up to a live process.  For example:

	echo 'print (char *)(0xdeadbeef+48)' | gdb -p $PID

Will show the pointer part of an SV
---
 Mwrap.xs                | 5 ++---
 lib/Devel/Mwrap/PSGI.pm | 7 ++++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Mwrap.xs b/Mwrap.xs
index 43d1ed6..bb51930 100644
--- a/Mwrap.xs
+++ b/Mwrap.xs
@@ -1049,9 +1049,7 @@ CODE:
 	++locating;
 	rcu_read_lock();
 	cds_list_for_each_entry_rcu(h, &self->allocs, anode) {
-		size_t gen = uatomic_read(&h->as.live.gen);
 		size_t size = uatomic_read(&h->size);
-
 		if (size > min) {
 			dSP;
 			ENTER;
@@ -1065,7 +1063,8 @@ CODE:
 			 */
 			XPUSHs(arg);
 			XPUSHs(sv_2mortal(newSVuv(size)));
-			XPUSHs(sv_2mortal(newSVuv(gen)));
+			XPUSHs(sv_2mortal(newSVuv(h->as.live.gen)));
+			XPUSHs(sv_2mortal(newSVuv((uintptr_t)h->real)));
 			PUTBACK;
 
 			call_sv(cb, G_DISCARD|G_EVAL);
diff --git a/lib/Devel/Mwrap/PSGI.pm b/lib/Devel/Mwrap/PSGI.pm
index 52dbebe..12bb929 100644
--- a/lib/Devel/Mwrap/PSGI.pm
+++ b/lib/Devel/Mwrap/PSGI.pm
@@ -121,8 +121,9 @@ EOM
 }
 
 sub each_at_i {
-	my ($fh, $size, $gen) = @_;
-	print $fh "<tr><td>$size</td><td>$gen</td></tr>\n";
+	my ($fh, $size, $gen, $addr) = @_;
+	print $fh "<tr><td>$size</td><td>$gen</td><td>";
+	printf $fh "0x%lx</td></tr>\n", $addr;
 }
 
 sub each_at {
@@ -134,7 +135,7 @@ sub each_at {
 	print $fh <<EOM;
 <html><head><title>$t</title></head><body><p>live allocations at $t
 <p>Current age: $age (live: $live)\n<table>
-<tr><th>size</th><th>generation</th></tr>
+<tr><th>size</th><th>generation</th><th>address</th></tr>
 EOM
 	$src_loc->each(0, \&each_at_i, $fh);
 	print $fh "</table></body></html>\n";