Bug 2091167 - SizeLimit plugin parses all of /proc/self/smaps in pure Perl after every request (~3 ms per request)

Bugzilla/App/Plugin/SizeLimit.pm

@@ -11,14 +11,11 @@ use Mojo::Base 'Mojolicious::Plugin';
 use Mojo::JSON qw(decode_json);
 use Bugzilla::Logging;
 
-use constant MIN_SIZE_LIMIT        => 750_000;
-use constant HAVE_LINUX_SMAPS_TINY => eval { require Linux::Smaps::Tiny };
-use constant HAVE_BSD_RESOURCE     => eval { require BSD::Resource };
+use constant MIN_SIZE_LIMIT    => 750_000;
+use constant PROC_STATUS       => '/proc/self/status';
+use constant HAVE_BSD_RESOURCE => eval { require BSD::Resource };
 
 BEGIN {
-  if (HAVE_LINUX_SMAPS_TINY) {
-    Linux::Smaps::Tiny->import('get_smaps_summary');
-  }
   if (HAVE_BSD_RESOURCE) {
     BSD::Resource->import;
   }
@@ -54,7 +51,7 @@ sub register {
     });
   }
 
-  if (HAVE_LINUX_SMAPS_TINY) {
+  if (defined vm_size_kb()) {
     my $size_limit = Bugzilla->localconfig->size_limit;
     return unless $size_limit;
 
@@ -65,11 +62,11 @@ sub register {
 
     $app->hook(
       after_dispatch => sub {
-        my $c       = shift;
-        my $summary = get_smaps_summary();
-        if ($summary->{Size} >= $size_limit) {
-          my $diff = $summary->{Size} - $size_limit;
-          INFO("memory size exceeded $size_limit by $diff ($summary->{Size})");
+        my $c    = shift;
+        my $size = vm_size_kb() // return;
+        if ($size >= $size_limit) {
+          my $diff = $size - $size_limit;
+          INFO("memory size exceeded $size_limit by $diff ($size)");
           $c->res->headers->connection('close');
           Mojo::IOLoop->singleton->stop_gracefully;
         }
@@ -78,4 +75,18 @@ sub register {
   }
 }
 
+# Virtual memory size in kB from VmSize, or undef if unavailable (non-Linux).
+sub vm_size_kb {
+  open my $fh, '<', PROC_STATUS or return undef;
+  my $size;
+  while (my $line = <$fh>) {
+    if ($line =~ /^VmSize:\s+(\d+)\s+kB/) {
+      $size = $1;
+      last;
+    }
+  }
+  close $fh;
+  return $size;
+}
+
 1;

Makefile.PL

@@ -283,12 +283,9 @@ my %optional_features = (
     description => 'Linux::PID',
     prereqs     => {runtime => {requires => {'Linux::Pid' => 0},},},
   },
-  linux_smaps => {
-    description => 'Linux::Smaps::Tiny for limiting memory usage',
-    prereqs     => {
-      runtime =>
-        {requires => {'Linux::Smaps::Tiny' => '0', 'BSD::Resource' => 0}}
-    },
+  rlimit => {
+    description => 'BSD::Resource for limiting memory usage',
+    prereqs     => {runtime => {requires => {'BSD::Resource' => 0}}},
   },
   linux_pdeath => {
     description => 'Linux::Pdeathsig for a good parent/child relationships',

README.md

@@ -203,8 +203,8 @@ This is a JSON object and can set any limit described in [BSD::Resource](https:/
 Typically it used for setting RLIMIT_AS, and the default value is `{ "RLIMIT_AS": 2000000000 }`.
 
 **BMO_size_limit**  
-This is the max amount of unshared memory the worker processes are allowed to
-use before they will exit. Minimum 750000 (750MiB)
+This is the max virtual memory size (VmSize, in kB) the worker processes are
+allowed to use before they will exit. Minimum 750000 (750MiB)
 
 **BMO_mail_delivery_method**  
 Usually configured on the MTA section of admin interface, but may be set here for testing purposes.

cpanfile

@@ -71,7 +71,6 @@ requires 'LWP::UserAgent', '6.44';
 requires 'LWP::UserAgent::Determined';
 requires 'Linux::Pdeathsig';
 requires 'Linux::Pid';
-requires 'Linux::Smaps::Tiny';
 requires 'List::MoreUtils', '0.418';
 requires 'Log::Dispatch', '2.67';
 requires 'Log::Log4perl', '1.49';

t/app-size-limit.t

@@ -0,0 +1,58 @@
+#!/usr/bin/env perl
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+use strict;
+use warnings;
+use 5.10.1;
+use lib qw( . lib local/lib/perl5 );
+
+BEGIN { $ENV{LOG4PERL_CONFIG_FILE} = 'log4perl-t.conf' }
+
+# Empty setrlimit so the plugin does not cap this test process's memory.
+use Bugzilla::Test::MockLocalconfig (size_limit => 750_000, setrlimit => '{}');
+use Bugzilla;
+use Bugzilla::App::Plugin::SizeLimit;
+use Test2::V0;
+use Test::Mojo;
+use Mojolicious::Lite;
+
+subtest 'vm_size_kb reads VmSize' => sub {
+  skip_all 'no /proc/self/status' unless -r '/proc/self/status';
+  my $size = Bugzilla::App::Plugin::SizeLimit::vm_size_kb();
+  like($size, qr/^\d+$/, 'whole number of kB');
+  ok($size > 1024, "plausible size ($size kB)");
+};
+
+subtest 'after_dispatch closes the worker only when over the limit' => sub {
+  skip_all 'no /proc/self/status' unless -r '/proc/self/status';
+
+  # Must be defined when register() runs, or it skips installing the hook.
+  my $fake_size = 1;
+  my $stops     = 0;
+  my $mock_size = mock 'Bugzilla::App::Plugin::SizeLimit' =>
+    (override => [vm_size_kb => sub {$fake_size}]);
+  my $mock_loop
+    = mock 'Mojo::IOLoop' => (override => [stop_gracefully => sub { $stops++ }]);
+
+  plugin 'Bugzilla::App::Plugin::SizeLimit';
+  get '/' => sub { shift->render(text => 'ok') };
+  my $t = Test::Mojo->new;
+
+  $fake_size = 749_999;
+  $t->get_ok('/')->status_is(200)->header_isnt(Connection => 'close');
+  is($stops, 0, 'under the limit: worker keeps running');
+
+  $fake_size = 750_000;
+  $t->get_ok('/')->status_is(200)->header_is(Connection => 'close');
+  is($stops, 1, 'at the limit: worker stops gracefully');
+
+  $fake_size = undef;
+  $t->get_ok('/')->status_is(200)->header_isnt(Connection => 'close');
+  is($stops, 1, 'size unavailable: no action');
+};
+
+done_testing;

template/en/default/setup/strings.txt.pl

@@ -280,7 +280,7 @@ END
 function. The default sets RLIMIT_AS to 2GiB.
 EOT
   localconfig_size_limit => <<EOT,
-This is the max amount of unshared memory the worker processes are allowed to use before they will exit.
+This is the max virtual memory size (VmSize, in kB) the worker processes are allowed to use before they will exit.
 EOT
   localconfig_shadowdb_user => <<EOT,
 The username used to authenticate to the shadow db.