Skip to content

Commit

Permalink
Prevent race condition in TFClient (RobotWebTools#489)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jorgenfb authored and breqdev committed Jan 8, 2022
1 parent a809e07 commit ca94679
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 0 deletions.
7 changes: 7 additions & 0 deletions src/tf/TFClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ function TFClient(options) {
this.frameInfos = {};
this.republisherUpdateRequested = false;
this._subscribeCB = null;
this._isDisposed = false;

// Create an Action client
this.actionClient = new ActionClient({
Expand Down Expand Up @@ -143,6 +144,12 @@ TFClient.prototype.updateGoal = function () {
* @param response the service response containing the topic name
*/
TFClient.prototype.processResponse = function (response) {
// Do not setup a topic subscription if already disposed. Prevents a race condition where
// The dispose() function is called before the service call receives a response.
if (this._isDisposed) {
return;
}

// if we subscribed to a topic before, unsubscribe so
// the republisher stops publishing it
if (this.currentTopic) {
Expand Down
30 changes: 30 additions & 0 deletions test/tfclient.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
var expect = require('chai').expect;
var ROSLIB = require('..');

describe('TFClient', function() {

describe('dispose', function() {

it('should not subscribe to republished topic if already disposed', function() {
// This test makes sure we do not subscribe to the republished topic if the
// tf client has already been disposed when we get the response (from the setup request)
// from the server.

var dummyROS = {
idCounter: 0,
on: () => {},
off: () => {},
callOnConnection: () => {}
}

var tfclient = new ROSLIB.TFClient({ros: dummyROS});
tfclient.dispose();

// Simulated a response from the server after the client is already disposed
tfclient.processResponse({topic_name: "/repub_1"});

expect(tfclient.currentTopic).to.be.false;
});
});

});

0 comments on commit ca94679

Please sign in to comment.