Skip to content

Commit

Permalink
fix GuiPlugin object leakage (#230)
Browse files Browse the repository at this point in the history
  • Loading branch information
@oetiker
oetiker authored Jun 25, 2024
1 parent 6c4ebde commit 7d97bde
Show file tree
Hide file tree
Showing 9 changed files with 155 additions and 70 deletions.
4 changes: 4 additions & 0 deletions CHANGES
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
0.49.4 2024-06-12 11:11:06 +0200 Tobias Oetiker <[email protected]>

- resolve gui object leakage this will result in smaller processes!

0.49.3 2024-02-27 14:22:44 +0100 Tobias Oetiker <[email protected]>

- Update required flag on change of required property
Expand Down
14 changes: 9 additions & 5 deletions lib/CallBackery.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ use CallBackery::Config;
use CallBackery::Plugin::Doc;
use CallBackery::Database;
use CallBackery::User;
use Scalar::Util qw(weaken);

our $VERSION = '0.49.3';
our $VERSION = '0.49.4';

=head2 config
Expand All @@ -63,12 +64,15 @@ An instance of L<CallBackery::Database> or a module with the same API.
=cut

has 'database' => sub {
has database => sub {
CallBackery::Database->new(app=>shift);
};

has 'userObject' => sub {
CallBackery::User->new();
has userObject => sub {
my $app = shift;
my $ obj = CallBackery::User->new(app=>$app,log=>$app->log);
$obj->{prototype} = 1;
return $obj;
};

=head2 securityHeaders
Expand Down Expand Up @@ -131,7 +135,7 @@ sub startup {
# when things get converted to string and back
setlocale(LC_NUMERIC, "C");
setlocale(LC_TIME, "C");

weaken($app);
$app->config->postProcessCfg();
my $gcfg = $app->config->cfgHash->{BACKEND};
if ($gcfg->{log_file}){
Expand Down
44 changes: 30 additions & 14 deletions lib/CallBackery/Config.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,12 @@ use Carp;
use autodie;
use File::Spec;
use Locale::PO;
use Mojo::Promise;
use Mojo::Loader qw(load_class);
use Mojo::JSON qw(true false);
use Mojo::Exception;
use Scalar::Util qw(blessed);
# use Devel::Cycle;

=head2 file
Expand All @@ -50,7 +53,7 @@ has secretFile => sub ($self) {
return $secretFile;
};

has app => sub { croak "the app parameter is mandatory" };
has app => sub { croak "the app parameter is mandatory" }, weak => 1;

has log => sub {
shift->app->log;
Expand All @@ -67,6 +70,8 @@ has cfgHash => sub {
my $cfg_file = shift;
my $parser = $self->makeParser();
my $cfg = $parser->parse($self->file, {encoding => 'utf8'}) or croak($parser->{err});
# the grammar is self referential, so we need to clean it up
$self->grammar(undef);
return $cfg;
};

Expand Down Expand Up @@ -149,14 +154,17 @@ sub loadAndNewPlugin {
if (my $e = load_class "${path}::$plugin") {
die mkerror(3894,"Loading ${path}::$plugin: $e") if ref $e;
} else {
return "${path}::${plugin}"->new();
my $proto = "${path}::${plugin}"->new(log=>$self->log);
$proto->{prototype} = 1;
return $proto;
}
}
die mkerror(123, "Plugin Module $plugin not found");
};

has grammar => sub {
my $self = shift;

my $pluginList = {};
my $pluginPath = $self->pluginPath;
for my $path (@INC){
Expand Down Expand Up @@ -375,7 +383,7 @@ sub postProcessCfg {
my $self = shift;
my $cfg = $self->cfgHash;
# only postprocess once
return $cfg if $cfg->{PLUGIN}{list};
return $cfg if $cfg->{PLUGIN}{list};
my %plugin;
my @pluginOrder;
for my $section (sort keys %$cfg){
Expand Down Expand Up @@ -429,28 +437,35 @@ sub _getPluginObject {
$name =~ s/[^-_0-9a-z]/_/gi;
die mkerror(39943,"No prototype for $name")
if not defined $prototype;

$prototype->new(
my $obj = $prototype->new(
user => $user,
name => $prototype->name,
config => $prototype->config,
args => $args // {},
app => $self->app,
);
$obj->log; # make sure logging is initialized
return $obj;
}

async sub instantiatePlugin_p {
# do not (!!!) implement this with async/await as it causes the the generated
# object to be somehow get a problem with reference counting with prevents
# timely destruction of the object 2024-06-12 tobi
sub instantiatePlugin_p {
my $self = shift;
my $obj = $self->_getPluginObject(@_);
my $name = $obj->name;
die mkerror(39944,"No permission to access $name")
if not await $self->promisify($obj->checkAccess);
return $obj;
return $self->promisify($obj->checkAccess)->then(sub {
my $access = shift;
return $obj if $access;
my $name = $obj->name;
Mojo::Promise->reject(mkerror(39944,"No permission to access $name"));
});
}

sub instantiatePlugin {
my $self = shift;
my $obj = $self->_getPluginObject(@_);
my @args = @_;
my $obj = $self->_getPluginObject(@args);
my $name = $obj->name;
die mkerror(39944,"No permission to access $name")
if not $self->promiseDeath($obj->checkAccess);
Expand All @@ -465,7 +480,7 @@ return the configuration state of the system as a blob

has configPlugins => sub {
my $self = shift;
my $user = $self->app->userObject->new(app=>$self->app,userId=>'__CONFIG');
my $user = $self->app->userObject->new(app=>$self->app,userId=>'__CONFIG', log=>$self->log);
my $cfg = $self->cfgHash;
my @plugins;
for my $name (@{$cfg->{PLUGIN}{list}}){
Expand Down Expand Up @@ -554,7 +569,7 @@ sub restoreConfigBlob {
$config = $self->unpack16($crypt->decrypt($config));

my $cfg = $self->cfgHash;
my $user = $self->app->userObject->new(app=>$self->app,userId=>'__CONFIG');
my $user = $self->app->userObject->new(app=>$self->app,userId=>'__CONFIG', log=>$self->log);
open my $fh ,'<', \$config;
my $zip = Archive::Zip->new();
$zip->readFromFileHandle($fh);
Expand Down Expand Up @@ -669,7 +684,8 @@ sub promisify {
if (eval { blessed $value && $value->isa('Mojo::Promise') }){
return $value;
}
return Mojo::Promise->resolve($value,@_);

return Mojo::Promise->resolve($value);
}

=head2 $cfg->promiseDeath(xxx)
Expand Down
64 changes: 44 additions & 20 deletions lib/CallBackery/Controller/RpcService.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ use Mojo::Base qw(Mojolicious::Plugin::Qooxdoo::JsonRpcController),
-signatures,-async_await;
use CallBackery::Exception qw(mkerror);
use CallBackery::Translate qw(trm);
use Scalar::Util qw(blessed weaken);
use Mojo::JSON qw(encode_json decode_json from_json);
use Syntax::Keyword::Try;
use Scalar::Util qw(blessed weaken);

=head1 NAME
Expand Down Expand Up @@ -47,18 +47,17 @@ my %allow = (

has config => sub ($self) {
$self->app->config;
};
}, weak => 1;

has user => sub ($self) {
my $obj = $self->app->userObject->new(controller=>$self,log=>$self->log);
weaken $obj->{controller};
my $obj = $self->app->userObject->new(app=>$self->app,controller=>$self,log=>$self->log);
return $obj;
};

has pluginMap => sub ($self) {
my $map = $self->config->cfgHash->{PLUGIN};
return $map;
};
}, weak => 1;


sub allow_rpc_access ($self,$method) {
Expand Down Expand Up @@ -217,9 +216,9 @@ async sub login { ## no critic (RequireArgUnpacking)
my $login = shift;
my $password = shift;
my $cfg = $self->config->cfgHash->{BACKEND};
if (my $ok =
if (my $ok =
await $self->config->promisify($self->user->login($login,$password))){
return {
return {
sessionCookie => $self->user->makeSessionCookie()
}
} else {
Expand Down Expand Up @@ -257,6 +256,16 @@ async sub instantiatePlugin_p {
return $plugin;
}

sub instantiatePlugin {
my $self = shift;
my $name = shift;
my $args = shift;
my $user = $self->user;
my $plugin = $self->config->instantiatePlugin($name,$user,$args);
$plugin->log($self->log);
return $plugin;
}

=head2 processPluginData(plugin,args)
handle form sumissions
Expand Down Expand Up @@ -311,27 +320,28 @@ returns user specific configuration information
=cut


async sub getUserConfig {
my $self = shift;
my $args = shift;
my @plugins;
my $ph = $self->pluginMap;
for my $plugin (@{$ph->{list}}){
my $obj;
for my $plugin ($self->pluginMap->{list}->@*){
try {
$obj = await $self->instantiatePlugin_p($plugin,$args);
my $obj = await $self->instantiatePlugin_p($plugin,$args);
#$obj = $self->instantiatePlugin($plugin,$args);
# $self->log->debug("instanciate $plugin");
push @plugins, {
tabName => $obj->tabName,
name => $obj->name,
instantiationMode => $obj->instantiationMode
};
} catch ($error) {
warn "$error";
}
next unless $obj;
push @plugins, {
tabName => $obj->tabName,
name => $obj->name,
instantiationMode => $obj->instantiationMode
};
}
my $userConfig = await $self->config->promisify($self->user->userInfo);
return {
userInfo => await $self->config->promisify($self->user->userInfo),
userInfo => $userConfig,
plugins => \@plugins,
};
}
Expand Down Expand Up @@ -363,10 +373,10 @@ following events are known:
sub runEventActions {
my $self = shift;
my $event = shift;
my @args = @_;
for my $obj (@{$self->config->configPlugins}){
weaken $obj->controller($self)->{controller};
if (my $action = $obj->eventActions->{$event}){
$action->(@_)
$action->(@args)
}
}
}
Expand Down Expand Up @@ -548,6 +558,20 @@ async sub handleDownload {
$self->rendered(200);
}

sub DESTROY ($self) {
# we are only interested in objects that get destroyed during
# global destruction as this is a potential problem
my $class = ref($self) // "child of ". __PACKAGE__;
if (${^GLOBAL_PHASE} ne 'DESTRUCT') {
# $self->log->debug($class." DESTROYed");
return;
}
if (blessed $self && ref $self->log){
$self->log->debug("late destruction of $class object during global destruction");
return;
}
warn "extra late destruction of $class object during global destruction\n";
}

1;
__END__
Expand Down
3 changes: 1 addition & 2 deletions lib/CallBackery/Database.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use Mojo::Base -base,-signatures;
use Data::Dumper;
use Carp qw(croak);
use CallBackery::Exception qw(mkerror);
use Scalar::Util qw(weaken);

=head1 NAME
Expand Down Expand Up @@ -38,7 +37,7 @@ object needs access to the system config to get access to the database

has app => sub {
croak "app property is required";
};
}, weak => 1;

has userName => sub {
return "* no user *";
Expand Down
Loading

0 comments on commit 7d97bde

Please sign in to comment.