mojolicious / mojo

:sparkles: Mojolicious - Perl real-time web framework
https://mojolicious.org
Artistic License 2.0
2.67k stars 580 forks source link

Mojo::DynamicMethods does not seem reliable #1336

Closed marioroy closed 5 years ago

marioroy commented 5 years ago

Hi,

The self-variable in the monkey_patch anonymous block, inside the Mojo::DynamicMethods::import function, is not always defined. Yet, the code assumes $self is defined.

  monkey_patch $dyn_pkg, 'can', sub {
    my ($self, $method, @rest) = @_;     # <-- $self not always defined

    # Delegate to our parent's "can" if there is one, without breaking if not
    my $can = $self->$caller_can($method, @rest);
    return undef unless $can;
    no warnings 'once';
    my $h = do { no strict 'refs'; *{"${dyn_pkg}::${method}"}{CODE} };
    return $h && $h eq $can ? undef : $can;
  };

Our app runs fine if we override-disable Mojo::DynamicMethods::import with sub { }.

Regards, Mario

shadowcat-mst commented 5 years ago

I'm sorry? how ... is anything managing to call can() with no $self ? The signature of can() is $class->can($method_name) or $object->can($method_name).

Something in your call stack is definitely doing something weird - could you try re-enabling DynamicMethods and adding something like

Carp::cluck("Undefined \$self in can('${method}') call") unless defined($self);

and telling me what the backtrace says?

I genuinely can't think of what would end up calling this code in a way that wouldn't have an object or a class as a first argument, so I need more information if I'm going to find the problem.

marioroy commented 5 years ago

Ah. Met to say $self not blessed. Sorry about that.

    if ( ! ref($self) ) {
      Carp::cluck("Not blessed \$self in can('${method}') call");
      sleep 10;
    }

Here is the backtrace.

Not blessed $self in can('new') call at /usr/local/lib/perl5/site_perl/Mojo/DynamicMethods.pm line 17.
    Mojolicious::Controller::_Dynamic::can("EIG::Rosetta::API::Mojo::Controller", "new") called at /usr/local/lib/perl5/site_perl/Moo.pm line 150
    Moo::_accessor_maker_for("Moo", "EIG::Rosetta::API::Mojo::Controller") called at /usr/local/lib/perl5/site_perl/Moo.pm line 175
    Moo::_constructor_maker_for("Moo", "EIG::Rosetta::API::Mojo::Controller") called at /usr/local/lib/perl5/site_perl/Moo/Role.pm line 433
    Moo::Role::_handle_constructor("Moo::Role", "EIG::Rosetta::API::Mojo::Controller", "EIG::Role::HTTPStatus") called at /usr/local/lib/perl5/site_perl/Role/Tiny.pm line 272
    Role::Tiny::apply_roles_to_package("Moo::Role", "EIG::Rosetta::API::Mojo::Controller", "EIG::Role::HTTPStatus", "EIG::Rosetta::Role::Authentication", "EIG::Rosetta::Role::JSONSchema") called at /usr/local/lib/perl5/site_perl/Moo/Role.pm line 280
    Moo::Role::apply_roles_to_package("Moo::Role", "EIG::Rosetta::API::Mojo::Controller", "EIG::Role::HTTPStatus", "EIG::Rosetta::Role::Authentication", "EIG::Rosetta::Role::JSONSchema") called at /usr/local/lib/perl5/site_perl/Moo.pm line 59
    Moo::with("EIG::Role::HTTPStatus", "EIG::Rosetta::Role::Authentication", "EIG::Rosetta::Role::JSONSchema") called at /home/mroy/EIG/rosetta/lib/EIG/Rosetta/API/Mojo/Controller.pm line 16
    require EIG/Rosetta/API/Mojo/Controller.pm called at /usr/local/lib/perl5/site_perl/Module/Runtime.pm line 314
    Module::Runtime::require_module("EIG::Rosetta::API::Mojo::Controller") called at /usr/local/lib/perl5/site_perl/Module/Runtime.pm line 391
    eval {...} called at /usr/local/lib/perl5/site_perl/Module/Runtime.pm line 391
    Module::Runtime::use_package_optimistically("EIG::Rosetta::API::Mojo::Controller") called at /usr/local/lib/perl5/site_perl/Moo/_Utils.pm line 50
    Moo::_Utils::_load_module("EIG::Rosetta::API::Mojo::Controller") called at /usr/local/lib/perl5/site_perl/Moo.pm line 115
    Moo::_set_superclasses("Moo", "Gapps::Router", "EIG::Rosetta::API::Mojo::Controller") called at /usr/local/lib/perl5/site_perl/Moo.pm line 53
    Moo::extends("EIG::Rosetta::API::Mojo::Controller") called at /home/mroy/EIG/rosetta/server_instances/gapps/lib/Gapps/Router.pm line 4
    require Gapps/Router.pm called at (eval 1279) line 1
    eval 'require Gapps::Router; 1' called at /usr/local/lib/perl5/site_perl/Mojo/Loader.pm line 44
    Mojo::Loader::load_class("Gapps::Router") called at /usr/local/lib/perl5/site_perl/Mojolicious/Routes.pm line 190
    Mojolicious::Routes::_load(Mojolicious::Routes=HASH(0x804cfd4f8), "Gapps::Router") called at /usr/local/lib/perl5/site_perl/Mojolicious/Routes.pm line 132
    Mojolicious::Routes::_class(Mojolicious::Routes=HASH(0x804cfd4f8), Mojolicious::Controller=HASH(0x8083a7cf0), HASH(0x80834da98)) called at /usr/local/lib/perl5/site_perl/Mojolicious/Routes.pm line 149
    Mojolicious::Routes::_controller(Mojolicious::Routes=HASH(0x804cfd4f8), Mojolicious::Controller=HASH(0x8083a7cf0), HASH(0x80834da98), 1) called at /usr/local/lib/perl5/site_perl/Mojolicious/Routes.pm line 46
    Mojolicious::Routes::continue(Mojolicious::Routes=HASH(0x804cfd4f8), Mojolicious::Controller=HASH(0x8083a7cf0)) called at /usr/local/lib/perl5/site_perl/Mojolicious/Routes.pm line 54
    Mojolicious::Routes::dispatch(Mojolicious::Routes=HASH(0x804cfd4f8), Mojolicious::Controller=HASH(0x8083a7cf0)) called at /usr/local/lib/perl5/site_perl/Mojolicious.pm line 132
    Mojolicious::dispatch(Gapps=HASH(0x807677738), Mojolicious::Controller=HASH(0x8083a7cf0)) called at /usr/local/lib/perl5/site_perl/Mojolicious.pm line 142
    Mojolicious::__ANON__(undef, Mojolicious::Controller=HASH(0x8083a7cf0)) called at /usr/local/lib/perl5/site_perl/Mojolicious/Plugins.pm line 15
    Mojolicious::Plugins::__ANON__() called at /usr/local/lib/perl5/site_perl/Mojolicious.pm line 204
    eval {...} called at /usr/local/lib/perl5/site_perl/Mojolicious.pm line 204
    Mojolicious::_exception(CODE(0x8083e6cc0), Mojolicious::Controller=HASH(0x8083a7cf0)) called at /usr/local/lib/perl5/site_perl/Mojolicious/Plugins.pm line 15
    Mojolicious::Plugins::__ANON__() called at /usr/local/lib/perl5/site_perl/Mojolicious/Plugins.pm line 18
    Mojolicious::Plugins::emit_chain(Mojolicious::Plugins=HASH(0x8081e1a38), "around_dispatch", Mojolicious::Controller=HASH(0x8083a7cf0)) called at /usr/local/lib/perl5/site_perl/Mojolicious.pm line 147
    Mojolicious::handler(Gapps=HASH(0x807677738), Mojo::Transaction::HTTP=HASH(0x8083ac408)) called at /usr/local/lib/perl5/site_perl/Mojo/Server.pm line 68
    Mojo::Server::__ANON__(Mojo::Server::Daemon=HASH(0x8083a3a68), Mojo::Transaction::HTTP=HASH(0x8083ac408)) called at /usr/local/lib/perl5/site_perl/Mojo/EventEmitter.pm line 15
    Mojo::EventEmitter::emit(Mojo::Server::Daemon=HASH(0x8083a3a68), "request", Mojo::Transaction::HTTP=HASH(0x8083ac408)) called at /usr/local/lib/perl5/site_perl/Mojo/Server/Daemon.pm line 104
    Mojo::Server::Daemon::__ANON__(Mojo::Transaction::HTTP=HASH(0x8083ac408)) called at /usr/local/lib/perl5/site_perl/Mojo/EventEmitter.pm line 15
    Mojo::EventEmitter::emit(Mojo::Transaction::HTTP=HASH(0x8083ac408), "request") called at /usr/local/lib/perl5/site_perl/Mojo/Transaction/HTTP.pm line 60
    Mojo::Transaction::HTTP::server_read(Mojo::Transaction::HTTP=HASH(0x8083ac408), "POST /info/test/in_path HTTP/1.1\x{d}\x{a}X-EIG-provider: gapps\x{d}\x{a}Cont"...) called at /usr/local/lib/perl5/site_perl/Mojo/Server/Daemon.pm line 219
    Mojo::Server::Daemon::_read(Mojo::Server::Daemon=HASH(0x8083a3a68), "000576098c3e798733b70d6ad65036db", "POST /info/test/in_path HTTP/1.1\x{d}\x{a}X-EIG-provider: gapps\x{d}\x{a}Cont"...) called at /usr/local/lib/perl5/site_perl/Mojo/Server/Daemon.pm line 200
    Mojo::Server::Daemon::__ANON__(Mojo::IOLoop::Stream=HASH(0x8083a78a0)) called at /usr/local/lib/perl5/site_perl/Mojo/EventEmitter.pm line 15
    Mojo::EventEmitter::emit(Mojo::IOLoop::Stream=HASH(0x8083a78a0), "read", "POST /info/test/in_path HTTP/1.1\x{d}\x{a}X-EIG-provider: gapps\x{d}\x{a}Cont"...) called at /usr/local/lib/perl5/site_perl/Mojo/IOLoop/Stream.pm line 103
    Mojo::IOLoop::Stream::_read(Mojo::IOLoop::Stream=HASH(0x8083a78a0)) called at /usr/local/lib/perl5/site_perl/Mojo/IOLoop/Stream.pm line 51
    Mojo::IOLoop::Stream::__ANON__(Mojo::Reactor::Poll=HASH(0x8052901e0)) called at /usr/local/lib/perl5/site_perl/Mojo/Reactor/Poll.pm line 143
    eval {...} called at /usr/local/lib/perl5/site_perl/Mojo/Reactor/Poll.pm line 143
    Mojo::Reactor::Poll::_try(Mojo::Reactor::Poll=HASH(0x8052901e0), "I/O watcher", CODE(0x8083a9078), 0) called at /usr/local/lib/perl5/site_perl/Mojo/Reactor/Poll.pm line 58
    Mojo::Reactor::Poll::one_tick(Mojo::Reactor::Poll=HASH(0x8052901e0)) called at /usr/local/lib/perl5/site_perl/Mojo/Reactor/Poll.pm line 99
    Mojo::Reactor::Poll::start(Mojo::Reactor::Poll=HASH(0x8052901e0)) called at /usr/local/lib/perl5/site_perl/Mojo/IOLoop.pm line 134
    Mojo::IOLoop::start(Mojo::IOLoop=HASH(0x805cb0798)) called at /usr/local/lib/perl5/site_perl/Mojo/UserAgent.pm line 65
    Mojo::UserAgent::start(Mojo::UserAgent=HASH(0x8083a4a38), Mojo::Transaction::HTTP=HASH(0x8083a4be8)) called at /usr/local/lib/perl5/site_perl/Test/Mojo.pm line 382
    Test::Mojo::_request_ok(Test::Mojo=HASH(0x806811ea0), Mojo::Transaction::HTTP=HASH(0x8083a4be8), "/info/test/in_path") called at /usr/local/lib/perl5/site_perl/Test/Mojo.pm line 329
    Test::Mojo::_build_ok(Test::Mojo=HASH(0x806811ea0), "POST", "/info/test/in_path", HASH(0x806c1f420), "json", HASH(0x8083a4a20)) called at /usr/local/lib/perl5/site_perl/Test/Mojo.pm line 262
    Test::Mojo::post_ok(Test::Mojo=HASH(0x806811ea0), "/info/test/in_path", HASH(0x806c1f420), "json", HASH(0x8083a4a20)) called at t/Rosetta/010-basic/20-compile-params.t line 20
Grinnz commented 5 years ago

Calling methods on unblessed class names is normal.

jhthorsen commented 5 years ago

@marioroy: Why are you checking for ref($self), when your original statement was "is not always defined"? I think you should do this instead

# one of these
unless ($self) {
unless (defined $self) {
kraih commented 5 years ago

Please provide a minimal example application, otherwise i will have to assume you are doing something weird in your application code.

shadowcat-mst commented 5 years ago

I'm 99% sure I see the problem - I think this would happen if you've registered a helper called 'new' and the DynamicMethods can() is returning undef because of the "don't let helpers be returned from can()" feature.

Nothing to do with blessed/not blessed, if I'm right - but I'm not sure what the best fix is - since $class->new after this would call the helper under normal circumstances. I can see four possibilities here, but I'm not sure which one is the best:

1) Declare PEBKAC, do nothing 2) Since a class can never have helpers in that sense, have DynamicMethods just return the $caller_can result if !blessed($self) 3) Have the last line return $self->next::can instead of undef if there's a helper match (which I believe is what the original AUTOLOAD-with-no-can code would've done) 4) Barf if you try to register a helper with a name like 'new'

@marioroy can you confirm that you have a helper called 'new' ? if so, why?

marioroy commented 5 years ago

Hi,

We're baffled by this as much as you are. We're using our Moo-based Controller class, which has a built-in new(), and inheriting Mojolicious::Controller. The app works with Mojolicious up to 7.94. The only way we got the app to work with Mojolicious 8.12 was disabling Mojo::DynamicMethods::import.

I tried the two suggestions separately to no avail.

(2) return the $caller_can result if !blessed($self) (3) have the last line return $self->next::can instead of undef

shadowcat-mst commented 5 years ago

On Thu, Mar 14, 2019 at 02:23:08PM -0700, Mario Roy wrote:

Hi,

We're baffled by this as much as you are. We're using our Moo-based Controller class, which has a built-in new(), and inheriting Mojolicious::Controller. The app works with Mojolicious up to 7.94. The only way we got the app to work with Mojolicious 8.12 was disabling Mojo::DynamicMethods::import.

Ok. so.

1) I still need to know what helpers you have registered

2) You're really going to have to try a mini app with one Moo-based controller though it's moo doing $class->can('new') that's confusing that, and I think that problem is generic, no Moo-specific

(note: both Moo and DynamicMethods were originally my fault ... and I'm still confused ... so I can't say I'm surprised you are too)

-- Matt S Trout - Shadowcat Systems - Perl consulting with a commit bit and a clue

http://shadowcat.co.uk/blog/matt-s-trout/ http://twitter.com/shadowcat_mst/

Email me now on mst (at) shadowcat.co.uk and let's chat about how our CPAN commercial support, training and consultancy packages could help your team.

marioroy commented 5 years ago

1) There are zero helpers registered. We were hoping for a way to default to Perl native "can" behavior if nothing registered, not having $caller->can('SUPER::BUILD_DYNAMIC')?

2) Sure.

shadowcat-mst commented 5 years ago

Mojolicious apps always have helpers registered - DefaultHelpers and TagHelpers for a start. So (1) isn't really an option, sadly.

marioroy commented 5 years ago

Hi. We have a story to look further on our end, but it isn't prioritized yet and could be a while.

kraih commented 5 years ago

I suppose we don't have to prioritize this either then.

kraih commented 5 years ago

Will reopen this issue when actionable information is provided.

marioroy commented 5 years ago

This is work in progress as time permitted until finding the root cause. This weekend, shorten the hack. The Mojolicious application is a restful API service only.

package EIG::Rosetta::API::Mojo;

use Mojo::Base 'Mojolicious';
use Mojo::Log;

# Hack to run with Mojolicious 8.04 and later releases.
# Added 'app' method due to AUTOLOAD missing in 8.04+.
{
    no strict 'refs';
    # disable Mojo::DynamicMethods feature for Mojolicious::Controller only
    @{ "Mojolicious::Controller::ISA" } =
        grep { $_ !~ /::_Dynamic$/ } @{ "Mojolicious::Controller::ISA" };
}

sub app {
    my $self   = shift;
    my $helper = $self->renderer->get_helper('app');
    return $self->build_controller->$helper(@_);
}
# End hack

# This method will run once at server start
sub startup {
    ...
}

1;