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

Closing some Yarp ports from Node.js explicitly fails #59

Open
Tracked by #40
nunoguedelha opened this issue Nov 3, 2021 · 9 comments · Fixed by robotology/yarp.js#27
Open
Tracked by #40

Closing some Yarp ports from Node.js explicitly fails #59

nunoguedelha opened this issue Nov 3, 2021 · 9 comments · Fixed by robotology/yarp.js#27
Assignees
Labels
bug Something isn't working

Comments

@nunoguedelha
Copy link
Collaborator

We try to close all the YARP ports opened by the iCubTelemetryServer process, by running the following commands from Node.js (for instance via REPL):

> this.yarp.portHandler.close('/yarpjs/inertial:i')
> this.yarp.portHandler.close('/yarpjs/left_leg/stateExt:o')
> this.yarp.portHandler.close('/yarpjs/camLeftEye:i')
> this.yarp.portHandler.close('/yarpjs/camRightEye:i')
> this.yarp.portHandler.close('/yarpjs/left_leg/cartesianEndEffectorWrench:i')
> this.yarp.portHandler.close('/yarpjs/right_leg/cartesianEndEffectorWrench:i')

The last command hangs. The port is actually closed but the Javascript binding call never returns the hand to the terminal).

@nunoguedelha
Copy link
Collaborator Author

nunoguedelha commented Nov 3, 2021

The problem is specific to the port /yarpjs/right_leg/cartesianEndEffectorWrench:i as the failure occurs if it is the first we try to close.

It even crashes the terminal stdout.

@nunoguedelha
Copy link
Collaborator Author

Checks:

$ yarp exists /wholeBodyDynamics/right_leg/cartesianEndEffectorWrench:o /yarpjs/right_leg/cartesianEndEffectorWrench:i
[INFO] |yarp.os.Network| No connection from /wholeBodyDynamics/right_leg/cartesianEndEffectorWrench:o to /yarpjs/right_leg/cartesianEndEffectorWrench:i found

$ yarp name list | grep /yarpjs/right_leg/cartesianEndEffectorWrench
registration name /yarpjs/right_leg/cartesianEndEffectorWrench:i ip 192.168.1.18 port 10122 type tcp

For some unknown reason, calling directly port._close() works.

This is probably an old issue already happening when we were stopping the process with process.exit().

Analyzing...

@nunoguedelha
Copy link
Collaborator Author

I could not reproduce this issue anymore and didn't find anything relevant happening in https://github.com/robotology/yarp.js/blob/ceddda861b1b29478d3bc389d83765fed7297196/yarp.js#L231-L246.

So for now I'm closing the issue, until it happens again.

@nunoguedelha
Copy link
Collaborator Author

Reopened because it's frequently happening again when we are closing the telemetry server through a <CTRL+C> SIGINT.

@nunoguedelha
Copy link
Collaborator Author

On C++ bindings and C++ addons

yarp.js implementation uses a C++ addon (https://github.com/nodejs/worker/blob/master/doc/api/addons.md), YarpJS.node, a dynamically-linked shared object, written in C++, that can be found in the generated folder build/Release, and can be loaded into Node.js using the require() function:
https://github.com/robotology/yarp.js/blob/b15c8d4a7668eb0cac06a6e3954bb975fcd23b12/yarp.js#L2

Node.js Addons are dynamically-linked shared objects, written in C++, that can be loaded into Node.js using the require() function, and used just as if they were an ordinary Node.js module. They are used primarily to provide an interface between JavaScript running in Node.js and C/C++ libraries.

The filename extension of the compiled Addon binary is .node (as opposed to .dll or .so). The require() function is written to look for files with the .node file extension and initialize those as dynamically-linked libraries.

The YarpJS.node addon is implemented using the Native Abstractions for Node.js (or nan) API.

Non-blocking operations requirement

(https://github.com/nodejs/worker/blob/master/doc/api/addons.md))

Addon authors are encouraged to think about how to avoid blocking the Node.js event loop with I/O or other time-intensive tasks by off-loading work via libuv to non-blocking system operations, worker threads or a custom use of libuv's threads.

The idea is to delegate the ports closure to one of those worker threads, within a limited time, i.e. stopping that thread after a reasonable timeout and running a "yarp clean" afterwards if necessary.

@nunoguedelha
Copy link
Collaborator Author

The problem is specific to the port /yarpjs/right_leg/cartesianEndEffectorWrench:i as the failure occurs if it is the first we try to close.

Actually, the issue occurs also with /yarpjs/left_leg/cartesianEndEffectorWrench:i.

All connections are properly closed, during the server closure breakdown tasks, in

this.unlistenToNetworkPorts.forEach((disconnect) => {disconnect();}); // Disconnect all telemetry entries (asynch operation)
.

The issue occurs when the Node.js Event Loop thread calls the port closure through the following call tree:

_closeOnExit() --> _port.close() --> _port._close();

https://github.com/robotology/yarp.js/blob/b15c8d4a7668eb0cac06a6e3954bb975fcd23b12/yarp.js#L204-L208

https://github.com/robotology/yarp.js/blob/b15c8d4a7668eb0cac06a6e3954bb975fcd23b12/yarp.js#L233-L249

and the failure occurs when calling _port._close():
https://github.com/robotology/yarp.js/blob/b15c8d4a7668eb0cac06a6e3954bb975fcd23b12/yarp.js#L238:

The issue occurs in the YarpJS.node C++ addon and doesn't seem at all to be related to the code execution in yarp-openmct before that call.

Moving the issue to repository https://github.com/robotology/yarp.js.

@nunoguedelha
Copy link
Collaborator Author

nunoguedelha commented Nov 16, 2021

Couldn't move the issue. There is some problem with the conditions required to do it. for moving an issue from repo A to repo B:

  • you need write access to both repos.
  • Either they are in the same organisation, either they have the same owner.

@traversaro, I guess that "owner" means maintainer/admin? fact: I can move issues between this repo and robotology-superbuild. Are you admin/maintainer of https://github.com/robotology/yarp.js?

@nunoguedelha
Copy link
Collaborator Author

Proceeding the analysis here...

The simplest and fastest way to avoid this issue while looking for a proper solution is to remove all-together the exit listener which closes the ports. As the Node.js process using the ports is fully stopped with the process.exit() command, the ports are left behind unused and unattached to any process. they can be cleared out by a manual yarp clean from a terminal, but they can also be left alone with no harm to other processes. The telemetry server application can actually be run again and open the same ports without any "address conflict" issue.

Workaround implemented in robotology/yarp.js#27.

@nunoguedelha
Copy link
Collaborator Author

Will further analyze the issue and provide a fix later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
1 participant