mojolicious / mojo

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

Potential bug in how Mojo::IOLoop::Subprocess::run handles exit() from client code? #1555

Closed ebaudrez closed 3 years ago

ebaudrez commented 3 years ago

I'm trying to run code that calls exit() within a Mojo::IOLoop::Subprocess. This code illustrates my problem:

use Mojo::Base -strict;
use Test::More;
use Mojo::IOLoop;

my $err;
my $sub = Mojo::IOLoop->subprocess(
    sub { exit 55 },
    sub { my $self = shift; $err = shift; Mojo::IOLoop->stop }
);
Mojo::IOLoop->start;
cmp_ok $sub->exit_code, '==', 55, 'exit code';
is $err, '', 'no exception';

done_testing;

This code fails while testing the exception $err. Instead of the expected empty value, an exception is raised:

malformed JSON string, neither tag, array, object, number, string or atom, at character offset 0 (before "(end of string)")

The exception carries a reference to the JSON handling code in Mojo::JSON, but the actual issue is in Mojo::IOLoop::Subprocess::_start. The relevant lines are 52 and 53:

https://github.com/mojolicious/mojo/blob/f5e1c294d8b7c977a0f287b5e33a90257703a7b9/lib/Mojo/IOLoop/Subprocess.pm#L52 and https://github.com/mojolicious/mojo/blob/f5e1c294d8b7c977a0f287b5e33a90257703a7b9/lib/Mojo/IOLoop/Subprocess.pm#L53

At line 53, the output for the parent is serialized. However, this code doesn't have a chance to run if the child process calls exit(), as the exit is not caught by the eval on line 52.

Now I'm not sure whether you want to allow this usage, that's why I wrote "Potential bug" :) But if you want to allow exiting in a subprocess, this ought to be fixed. Let me know if you need more information.

Kind regards!

Grinnz commented 3 years ago

It expects there to always be a data structure returned. If you want to end the subprocess in a failure state, you could throw an exception as that is handled.

kraih commented 3 years ago

Nothing here looks like a bug to me, it's tested behaviour. If you want to propose a better way to deal with it you're welcome to do so of course.