Skip to content

Commit 6e8447c

Browse files
authored
Prevent race condition in TFClient (#489)
If a TFClient is disposed while a service call to update the goal is ongoing it will subscribe to the republished topic even if none is listening.
1 parent 9f5de34 commit 6e8447c

File tree

2 files changed

+38
-0
lines changed

2 files changed

+38
-0
lines changed

src/tf/TFClient.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ function TFClient(options) {
5252
this.frameInfos = {};
5353
this.republisherUpdateRequested = false;
5454
this._subscribeCB = null;
55+
this._isDisposed = false;
5556

5657
// Create an Action client
5758
this.actionClient = new ActionClient({
@@ -144,6 +145,12 @@ TFClient.prototype.updateGoal = function() {
144145
* @param response the service response containing the topic name
145146
*/
146147
TFClient.prototype.processResponse = function(response) {
148+
// Do not setup a topic subscription if already disposed. Prevents a race condition where
149+
// The dispose() function is called before the service call receives a response.
150+
if (this._isDisposed) {
151+
return;
152+
}
153+
147154
// if we subscribed to a topic before, unsubscribe so
148155
// the republisher stops publishing it
149156
if (this.currentTopic) {
@@ -216,6 +223,7 @@ TFClient.prototype.unsubscribe = function(frameID, callback) {
216223
* Unsubscribe and unadvertise all topics associated with this TFClient.
217224
*/
218225
TFClient.prototype.dispose = function() {
226+
this._isDisposed = true;
219227
this.actionClient.dispose();
220228
if (this.currentTopic) {
221229
this.currentTopic.unsubscribe(this._subscribeCB);

test/tfclient.test.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
var expect = require('chai').expect;
2+
var ROSLIB = require('..');
3+
4+
describe('TFClient', function() {
5+
6+
describe('dispose', function() {
7+
8+
it('should not subscribe to republished topic if already disposed', function() {
9+
// This test makes sure we do not subscribe to the republished topic if the
10+
// tf client has already been disposed when we get the response (from the setup request)
11+
// from the server.
12+
13+
var dummyROS = {
14+
idCounter: 0,
15+
on: () => {},
16+
off: () => {},
17+
callOnConnection: () => {}
18+
}
19+
20+
var tfclient = new ROSLIB.TFClient({ros: dummyROS});
21+
tfclient.dispose();
22+
23+
// Simulated a response from the server after the client is already disposed
24+
tfclient.processResponse({topic_name: "/repub_1"});
25+
26+
expect(tfclient.currentTopic).to.be.false;
27+
});
28+
});
29+
30+
});

0 commit comments

Comments
 (0)