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

Hang up with 'advanced' IPC deserialization #55834

Closed
MGorkov opened this issue Nov 13, 2024 · 4 comments · Fixed by #56106
Closed

Hang up with 'advanced' IPC deserialization #55834

MGorkov opened this issue Nov 13, 2024 · 4 comments · Fixed by #56106
Labels
needs more info Issues without a valid reproduction.

Comments

@MGorkov
Copy link
Contributor

MGorkov commented Nov 13, 2024

Version

20.12.2

Platform

RHEL 8.4

Subsystem

child_process

What steps will reproduce the bug?

I start a child process using fork() and { serialization: 'advanced' }. Then transfer messages between the child and the parent process.
Sometimes the transfer hangs at the receiving side.

How often does it reproduce? Is there a required condition?

Rarely, after several hours of high rate messaging

What is the expected behavior? Why is that the expected behavior?

Transmitting messages without hangups

What do you see instead?

The queue (kMessageBuffer) on the receiving side is constantly growing without processing.
The length of the first buffer in the kMessageBuffer is less than 4 bytes, so new messages are not processed and are only added to the end of queue.
image

Additional information

I suppose that this PR led to this behavior.
Instead of concatenating the queue and then checking the length, only the first buffer is checked.
This does not take into account that the first buffer may be smaller than 4 bytes after processing the previous message (code)
If the first buffer is less than 4 bytes, it must be concat with with the next one, if there is one.

@avivkeller
Copy link
Member

Can you provide a minimal reproduction?

@MGorkov
Copy link
Contributor Author

MGorkov commented Nov 13, 2024

Minimal reproduction:
test-send.zip

node main.js

image

@pmarchini
Copy link
Member

Could you please provide a repository or a minimal repro that doesn't require a download?

@avivkeller avivkeller added the needs more info Issues without a valid reproduction. label Nov 13, 2024
@MGorkov
Copy link
Contributor Author

MGorkov commented Nov 13, 2024

repo for repro

MGorkov added a commit to MGorkov/node that referenced this issue Dec 2, 2024
MGorkov added a commit to MGorkov/node that referenced this issue Dec 2, 2024
nodejs-github-bot pushed a commit that referenced this issue Jan 18, 2025
Fixes: #55834
PR-URL: #56106
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this issue Jan 27, 2025
Fixes: #55834
PR-URL: #56106
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this issue Jan 30, 2025
Fixes: #55834
PR-URL: #56106
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this issue Jan 31, 2025
Fixes: #55834
PR-URL: #56106
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this issue Feb 4, 2025
Fixes: #55834
PR-URL: #56106
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 pushed a commit that referenced this issue Feb 6, 2025
Fixes: #55834
PR-URL: #56106
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs more info Issues without a valid reproduction.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants