Prevent race condition in TFClient

[jorgenfb]

Public API Changes
None

Description
If a TFClient is disposed while a service call to update the goal is ongoing it will subscribe to the republished topic even if already disposed. To prevent this we mark the TFClient as disposed when it is disposed, and check for this in the service callback.

Fixes #488

[MatthijsBurgh]

@jorgenfb, could we create a test, which fails in the old situation, but works with the fix?

[jorgenfb]

I have added a tests that fails without my fix and also rebased on develop.

[MatthijsBurgh]

The karma:test step fails locally on my computer. While it always showed the same results as CI. So I can't accept this change for now. The test needs to be more robust.

Running "karma:test" (karma) task
07 01 2022 08:48:19.965:INFO [karma-server]: Karma v6.3.9 server started at http://localhost:9876/
07 01 2022 08:48:19.966:INFO [launcher]: Launching browsers Firefox with concurrency unlimited
07 01 2022 08:48:19.969:INFO [launcher]: Starting browser Firefox
07 01 2022 08:48:23.434:INFO [Firefox 95.0 (Ubuntu 0.0.0)]: Connected on socket 9K9kfopYzzASTtETAAAB with id 60949544
WARN: 'CBOR 64-bit integer array values may lose precision. No further warnings.'
ERROR: '(node) warning: possible EventEmitter memory leak detected. 11 listeners added. Use emitter.setMaxListeners() to increase limit.'
Firefox 95.0 (Ubuntu 0.0.0) TFClient dispose should not subscribe to republished topic if already disposed FAILED
	expected { Object (ros, name, ...) } to be false
	AssertionError@/home/matthijs/src/roslibjs/node_modules/chai/chai.js:9574:13
	[3]</module.exports/Assertion.prototype.assert@/home/matthijs/src/roslibjs/node_modules/chai/chai.js:250:13
	[5]</module.exports/<@/home/matthijs/src/roslibjs/node_modules/chai/chai.js:1072:10
	propertyGetter@/home/matthijs/src/roslibjs/node_modules/chai/chai.js:7959:29
	proxyGetter@/home/matthijs/src/roslibjs/node_modules/chai/chai.js:9372:22
	@tfclient.test.js:26:13
	
Firefox 95.0 (Ubuntu 0.0.0): Executed 25 of 25 (1 FAILED) (0.063 secs / 0.022 secs)
TOTAL: 1 FAILED, 24 SUCCESS
Warning: Task "karma:test" failed. Use --force to continue.

Aborted due to warnings.

edit: I didn't build yet. The mocha tests run on the source files, but the karma tests run on the builded files. Sorry for the confusion

[MatthijsBurgh]

@jorgenfb I didn't compile yet. Which caused the karma test to fail. Please restore the PR. Sorry for the confusion.

[jorgenfb]

Hi Matthijs! Can you please check locally again. I don't see why this test would be flaky, I have gone through the logic, and it is pretty simple. I just reverted my fix, without removing the test, and the CI clearly fails now.

I have also tried running this on my machine:

docker build --build-arg ROS_DISTRO=melodic --build-arg NODE_VERSION=14 -t roslibjsdocker .
docker run -v $(pwd):/root/roslibjs --rm roslibjsdocker bash -i -c 'bash /root/roslibjs/test/build.bash'

It works

[jorgenfb]

Good! I will remove the last commit again. No problem, these things happen