Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proc::Async methods can have unpredictable and nondeterministic order #4669

Open
p6rt opened this issue Oct 22, 2015 · 4 comments
Open

Proc::Async methods can have unpredictable and nondeterministic order #4669

p6rt opened this issue Oct 22, 2015 · 4 comments
Labels

Comments

@p6rt
Copy link

p6rt commented Oct 22, 2015

Migrated from rt.perl.org#126425 (status was 'open')

Searchable as RT126425$

@p6rt
Copy link
Author

p6rt commented Oct 22, 2015

From @hoelzro

Consider the following two programs​:

parent.pl​:

  my $child = Proc​::Async.new('perl6', 'child.pl', :w);
  $child.stdout.tap​: { .say };
  $child.print​: "1\n";
  $child.kill​: SIGINT;
  $child.print​: "2\n";
 
child.pl​:

  signal(SIGINT).act​: { say 'SIGINT'; exit }

  my $ = get();
  say 'one';
  my $ = get();
  say 'two';
  say 'done';

There are seven Proc​::Async related events that occur when you run parent.pl​:

  - "1\n" is sent to the child on standard input.
  - "one\n" is received from the child on standard output.
  - SIGINT is sent to the child.
  - "SIGINT\n" is received from the child on standard output.
  - "2\n" is sent to the child on standard input.
  - "two\n" is received from the child on standard output.
  - "done\n" is received from the child on standard output.

If this program behaved correctly, not all of these events would occur, but due to the way these methods are implemented, the ordering may change. 7! is 5040, but I wrote a little program to prune orderings that just can't happen (ex. the child will always receive "1\n" before "2\n"). That program indicates that there are 42 possible orderings; I haven't observed so many when running this program, but I have observed a few.

I understand that Proc​::Async is asynchronous in all its dealings (hence the name), but the way the code looks feels very synchronous. Either the nondeterministic orderings should be fixed, or maybe the API should change to reflect that here be dragons.

This affects S17-procasync/kill.t.

@p6rt
Copy link
Author

p6rt commented Oct 22, 2015

From @moritz

On Wed Oct 21 20​:03​:30 2015, rob@​hoelz.ro wrote​:

Consider the following two programs​:

parent.pl​:

my $child = Proc​::Async.new('perl6', 'child.pl', :w);
$child.stdout.tap​: { .say };
$child.print​: "1\n";
$child.kill​: SIGINT;
$child.print​: "2\n";

child.pl​:

signal(SIGINT).act​: { say 'SIGINT'; exit }

my $ = get();
say 'one';
my $ = get();
say 'two';
say 'done';

There are seven Proc​::Async related events that occur when you run
parent.pl​:

- "1\n" is sent to the child on standard input.
- "one\n" is received from the child on standard output.
- SIGINT is sent to the child.
- "SIGINT\n" is received from the child on standard output.
- "2\n" is sent to the child on standard input.
- "two\n" is received from the child on standard output.
- "done\n" is received from the child on standard output.

If this program behaved correctly, not all of these events would
occur, but due to the way these methods are implemented, the ordering
may change. 7! is 5040, but I wrote a little program to prune
orderings that just can't happen (ex. the child will always receive
"1\n" before "2\n"). That program indicates that there are 42
possible orderings; I haven't observed so many when running this
program, but I have observed a few.

I understand that Proc​::Async is asynchronous in all its dealings
(hence the name), but the way the code looks feels very synchronous.
Either the nondeterministic orderings should be fixed, or maybe the
API should change to reflect that here be dragons.

This affects S17-procasync/kill.t.

Sorry, but this is too vague for a bug report. "feels very synchronous" is vague, and it's not clear to me what needs to happen to close this ticket.

Note that write, print and say returns promises which you can await on to enforce ordering.

Please give some actual examples for things you consider bugs. Otherwise I'll close the ticket.

@p6rt
Copy link
Author

p6rt commented Oct 22, 2015

The RT System itself - Status changed from 'new' to 'open'

@p6rt
Copy link
Author

p6rt commented Oct 22, 2015

From @hoelzro

On 2015-10-21 21​:41​:40, moritz wrote​:

On Wed Oct 21 20​:03​:30 2015, rob@​hoelz.ro wrote​:

Consider the following two programs​:

parent.pl​:

my $child = Proc​::Async.new('perl6', 'child.pl', :w);
$child.stdout.tap​: { .say };
$child.print​: "1\n";
$child.kill​: SIGINT;
$child.print​: "2\n";

child.pl​:

signal(SIGINT).act​: { say 'SIGINT'; exit }

my $ = get();
say 'one';
my $ = get();
say 'two';
say 'done';

There are seven Proc​::Async related events that occur when you run
parent.pl​:

- "1\n" is sent to the child on standard input.
- "one\n" is received from the child on standard output.
- SIGINT is sent to the child.
- "SIGINT\n" is received from the child on standard output.
- "2\n" is sent to the child on standard input.
- "two\n" is received from the child on standard output.
- "done\n" is received from the child on standard output.

If this program behaved correctly, not all of these events would
occur, but due to the way these methods are implemented, the ordering
may change. 7! is 5040, but I wrote a little program to prune
orderings that just can't happen (ex. the child will always receive
"1\n" before "2\n"). That program indicates that there are 42
possible orderings; I haven't observed so many when running this
program, but I have observed a few.

I understand that Proc​::Async is asynchronous in all its dealings
(hence the name), but the way the code looks feels very synchronous.
Either the nondeterministic orderings should be fixed, or maybe the
API should change to reflect that here be dragons.

This affects S17-procasync/kill.t.

Sorry, but this is too vague for a bug report. "feels very
synchronous" is vague, and it's not clear to me what needs to happen
to close this ticket.

Note that write, print and say returns promises which you can await on
to enforce ordering.

Please give some actual examples for things you consider bugs.
Otherwise I'll close the ticket.

To me, this feels like a violation of "Similar things should look similar, different things should like different". If I'm a newbie, and I see this​:

  $child.print​: "1\n";
  $child.kill​: SIGINT;
  $child.print​: "2\n";

I will probably think "this is going to print '1' to the child, then send it SIGINT, then print '2' to the child. Unless you wait for the promises (which is the right thing to do here), you have several potential orderings of these three operations​:

  - print(1), kill(SIGINT), print(2)
  - print(1), print(2), kill(SIGINT)
  - kill(SIGINT), print(1), print(2)

(The other three permutations have print(2) before print(1), which can't happen with the current implementation, so I have omitted them)

So the problem (to me) is that this code reads like synchronous code, and I expected it to act with a deterministic order like synchronous code would. So I think the bug/resolution is one of the following​:

  1) The order of operations is nondeterministic - resolution would be to make them deterministic.
  2) The method calls look like synchronous method calls - resolution would be to change the names so it's obvious that they're synchronous and their order is not guaranteed, as well as what's required for resolution #​3 below.
  3) The example program (and S17-procasync/kill.t) is relying on undefined behavior (the order of Proc​::Async operations) - resolution would be to update kill.t to await the promises to enforce the expected order.

My issue with #​3 is that strange timing bugs can lurk in code that looks correct when you read it, and it's also easy to write code that conceals those same strange timing bugs. One solution for this would be to have print and friends fail in sink context (which strikes me as a bit magical), forcing you to acknowledge the Promise, but that would make the case where the user really *doesn't* care about operation order annoying to code. I personally favor #​2, changing print → async-print or something along those lines.

@p6rt p6rt added the conc label Jan 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant