Skip to content
This repository was archived by the owner on Apr 22, 2023. It is now read-only.

Conversation

@gireeshpunathil
Copy link
Member

simple/test-child-process-stdout-flush-exit.js fails with an assertion.
The root cause for this assertion is that the expected boolean value of
true for the variable gotBye was false. This is set to true when the
piped stdout stream of the child writes the end token "goodbye". So the
error message would indicate that the end token was never received by
the parent, but in fact it did. The only difference is that the first
chunk itself had both 'hello' and 'goodbye' (as well as the filler
words in between) in AIX, while Linux receives them separately.

While this issue is not reproducible in Linux, the number of bytes
received each time a callback is called is not consistent across runs,
which is ratified as the actual content size of a UNIX domain data packet
is determined outside of the node's logic, instead in OS tunables, as well
as the runtime context of data transfer (depending on contiguous free
memory available in OS data structures at the time of sending).
In addition, around 200 filler words sent in between the 'hello' and
'goodbye' seem to indicate that the coalescence of chunks was a possibility
in Linux as well, and was devised to separate the first word from the last,
through an arbitrary delimiter.

Parser logic seem to be rigid and have assumptions about the order and size
of the data arrival. For example, it checks for 'goodbye' only when it does
not find 'hello' in it, as if they would always come separately. This
exclusiveness is what makes the test to fail in AIX.

This pull request is to make provision in the test case to address
situations wherein the data arrives together, as well as to exhibit consistent
behavior irrespective of the order and content size of the data arrival.

@mhdawson mhdawson added this to the 0.12.3 milestone Apr 2, 2015
@mhdawson mhdawson self-assigned this Apr 2, 2015
@mhdawson mhdawson added the P-3 label Apr 2, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Styling comments:

  • No space between values/variables and opening/closing parenthesis, so:
    if ((0 == messageSequence ) becomes if ((0 == messageSequence).
  • Having rvalues on the left of equality operators is not common practice in the code base, as far as I know, so if ((0 == messageSequence ) would become if ((messageSequence === 0).

Also, always use strict equality === and not loose equality == unless it's not possible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We encourage the rvalues on the left because if you miss an = you end up with an error instead of assigning when you did not mean to. However, if that's not commonly done in the Node codebase I guess we should be consistent

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdawsonibm Right, my comment on the rvalue on the left of the equality operator was just to provide some additional context around common practices. I'm fine with keeping it, especially in a test file.

@misterdjules
Copy link

Thank you @gireeshpunathil!

One question: I thought that child_process used UNIX domain sockets for creating stdout, stderr and stdin objects, so my understanding is that the TCP protocol is not used for communication between a child and its parent. I may be missing something but if not, then we would need to adjust the commit message.

simple/test-child-process-stdout-flush-exit.js fails with an assertion.
The root cause for this assertion is that the expected boolean value of
true for the variable gotBye was false. This is set to true when the
piped stdout stream of the child writes the end token "goodbye". So the
error message would indicate that the end token was never received by
the parent, but in fact it did. The only difference is that the first
chunk itself had both 'hello' and 'goodbye' (as well as the filler
words in between) in AIX, while Linux receives them separately.

While this issue is not reproducible in Linux, the number of bytes
received each time a callback is called is not consistent across runs,
which is ratified as the actual content size of a UNIX domain data packet
is determined outside of the node's logic, instead in OS tunables, as well
as the runtime context of data transfer (depending on contigeous free
memory available in OS data structures at the time of sending).
In addition, around 200 filler words sent in between the 'hello' and
'goodbye' seem to indicate that the coalescence of chunks was a possibility
in Linux as well, and was devised to separate the first word from the last,
through an arbitrary delimiter.

Parser logic seem to be rigid and have assumptions about the order and size
of the data arrival. For example, it checks for 'goodbye' only when it does
not find 'hello' in it, as if they would always come separately. This
exclusiveness is what makes the test to fail in AIX.
@gireeshpunathil
Copy link
Member Author

@misterdjules, thanks for the review comments. Agreed that the UNIX domain sockets do not follow network protocols for data transfer. The chunk size seem to be partially tunable and partially runtime context dependent, which is manifested in test runs, both in Linux and AIX - the data chunk size changes between runs.

I have amended the commit message and the pull request description.

Also modified the code, according to your styling comments on the space, while left the rvalue stuff as it is, as I see both you and @mdawsonibm are in sync on the topic.

Thanks once again.

mhdawson pushed a commit that referenced this pull request Apr 13, 2015
simple/test-child-process-stdout-flush-exit.js fails with an assertion.
The root cause for this assertion is that the expected boolean value of
true for the variable gotBye was false. This is set to true when the
piped stdout stream of the child writes the end token "goodbye". So the
error message would indicate that the end token was never received by
the parent, but in fact it did. The only difference is that the first
chunk itself had both 'hello' and 'goodbye' (as well as the filler
words in between) in AIX, while Linux receives them separately.

While this issue is not reproducible in Linux, the number of bytes
received each time a callback is called is not consistent across runs,
which is ratified as the actual content size of a UNIX domain data packet
is determined outside of the node's logic, instead in OS tunables, as well
as the runtime context of data transfer (depending on contigeous free
memory available in OS data structures at the time of sending).
In addition, around 200 filler words sent in between the 'hello' and
'goodbye' seem to indicate that the coalescence of chunks was a possibility
in Linux as well, and was devised to separate the first word from the last,
through an arbitrary delimiter.

Parser logic seem to be rigid and have assumptions about the order and size
of the data arrival. For example, it checks for 'goodbye' only when it does
not find 'hello' in it, as if they would always come separately. This
exclusiveness is what makes the test to fail in AIX.

Reviewed-By:
PR-URL: #14410
@mhdawson
Copy link
Member

Landed as 4e154d6

@mhdawson mhdawson closed this Apr 13, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants