Merge pull request #158 from minggangw/fix-issue-123

Minggang Wang · web-flow · commit e41105a6a290 · 2017-10-30T10:19:09.000+08:00
[UT]Enable all tests on Linux/macOS/Windows
diff --git a/binding.gyp b/binding.gyp
@@ -34,6 +34,7 @@
         '-lrcl',
         '-lrcutils',
         '-lrmw',
+        '-lrosidl_generator_c',
       ],
       'conditions': [
         [
diff --git a/lib/client.js b/lib/client.js
@@ -57,22 +57,17 @@ class Client extends Entity {
       throw new TypeError('Invalid argument');
     }
 
-    rclRequest.serialize();
-    let rawROSRequest = rclRequest.toRawROS();
+    let rawROSRequest = rclRequest.serialize();
     this._sequenceNumber = rclnodejs.sendRequest(this._handle, rawROSRequest);
     this._callback = callback;
   }
 
-  get takenResponse() {
-    return this._response.toRawROS();
-  }
-
   get sequenceNumber() {
     return this._sequenceNumber;
   }
 
-  processResponse() {
-    this._response.deserialize();
+  processResponse(response) {
+    this._response.deserialize(response);
     this._callback(this._response);
   }
 
diff --git a/lib/entity.js b/lib/entity.js
@@ -33,6 +33,10 @@ class Entity {
   get qos() {
     return this._qos;
   }
+
+  get typeClass() {
+    return this._typeClass;
+  }
 }
 
 module.exports = Entity;
diff --git a/lib/node.js b/lib/node.js
@@ -54,24 +54,33 @@ class Node {
     });
 
     this._subscriptions.forEach((subscription) => {
-      let success = rclnodejs.rclTake(subscription.handle, subscription.takenMsg);
+      let Message = subscription.typeClass;
+      let msg = new Message();
+      let success = rclnodejs.rclTake(subscription.handle, msg.toRawROS());
       if (success) {
-        subscription.processResponse();
+        subscription.processResponse(msg.refObject);
       }
+      Message.destoryRawROS(msg);
     });
 
     this._clients.forEach((client) => {
-      let success = rclnodejs.rclTakeResponse(client.handle, client.sequenceNumber, client.takenResponse);
+      let Response = client.typeClass.Response;
+      let response = new Response();
+      let success = rclnodejs.rclTakeResponse(client.handle, client.sequenceNumber, response.toRawROS());
       if (success) {
-        client.processResponse();
+        client.processResponse(response.refObject);
       }
+      Response.destoryRawROS(response);
     });
 
     this._services.forEach((service) => {
-      let header = rclnodejs.rclTakeRequest(service.handle, this.handle, service.takenRequest);
+      let Request = service.typeClass.Request;
+      let request = new Request();
+      let header = rclnodejs.rclTakeRequest(service.handle, this.handle, request.toRawROS());
       if (header) {
-        service.processRequest(header);
+        service.processRequest(header, request.refObject);
       }
+      Request.destoryRawROS(request);
     });
   }
 
diff --git a/lib/publisher.js b/lib/publisher.js
@@ -50,8 +50,7 @@ class Publisher extends Entity {
       rclMessage.data = message;
     }
 
-    rclMessage.serialize();
-    let rawRosMessage = rclMessage.toRawROS();
+    let rawRosMessage = rclMessage.serialize();
     if (rawRosMessage) {
       rclnodejs.publish(this._handle, rawRosMessage);
     } else {
diff --git a/lib/service.js b/lib/service.js
@@ -30,15 +30,11 @@ class Service extends Entity {
     this._callback = callback;
   }
 
-  get takenRequest() {
-    return this._request.toRawROS();
-  }
-
-  processRequest(headerHandle) {
-    this._request.deserialize();
+  processRequest(headerHandle, request) {
+    this._request.deserialize(request);
     let response = this._callback(this._request, new this._typeClass.Response());
-    response.serialize();
-    let rawROSResponse = response.toRawROS();
+
+    let rawROSResponse = response.serialize();
     rclnodejs.sendResponse(this._handle, rawROSResponse, headerHandle);
   }
 
diff --git a/lib/subscription.js b/lib/subscription.js
@@ -30,12 +30,8 @@ class Subscription extends Entity {
     this._message = new typeClass();
   }
 
-  get takenMsg() {
-    return this._message.toRawROS();
-  }
-
-  processResponse() {
-    this._message.deserialize();
+  processResponse(msg) {
+    this._message.deserialize(msg);
     this._callback(this._message);
   }
 
diff --git a/rosidl_gen/deallocator.js b/rosidl_gen/deallocator.js
@@ -0,0 +1,25 @@
+// Copyright (c) 2017 Intel Corporation. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+'use strict';
+
+const rclnodejs = require('bindings')('rclnodejs');
+
+let deallocator = {
+  freeStructMember(refObj, type, name) {
+    rclnodejs.freeMemeoryAtOffset(refObj.ref(), type.fields[name].offset);
+  }
+};
+
+module.exports = deallocator;
diff --git a/rosidl_gen/generator_primitive.js b/rosidl_gen/generator_primitive.js
@@ -26,14 +26,21 @@ const StringRefStruct = StructType({
   capacity: ref.types.size_t,
 });
 
-function initString(str) {
-  if (! str instanceof StringRefStruct) {
-    throw new TypeError('Invalid argument: should provide a type of StringRefStruct');
-  }
+function initString(str, own = false) {
+  if (own) {
+    if (! str instanceof Buffer) {
+      throw new TypeError('Invalid argument: should provide a Node Buffer to bindingsStringInit()');
+    }
+    rclnodejs.initString(str);
+  } else {
+    if (! str instanceof StringRefStruct) {
+      throw new TypeError('Invalid argument: should provide a type of StringRefStruct');
+    }
 
-  str.data = Buffer.alloc(1).fill('\u0000');
-  str.size = 0;
-  str.capacity = 1;
+    str.data = Buffer.alloc(1).fill('\u0000');
+    str.size = 0;
+    str.capacity = 1;
+  }
 }
 
 module.exports = {
diff --git a/rosidl_gen/templates/message.dot b/rosidl_gen/templates/message.dot
@@ -9,6 +9,7 @@ const StructType = require('ref-struct');
 const ref = require('ref');
 const ArrayType = require('ref-array');
 const primitiveTypes = require('../../rosidl_gen/generator_primitive.js');
+const deallocator = require('../../rosidl_gen/deallocator.js');
 
 {{
 let objectWrapper = it.spec.msgName + 'Wrapper';
@@ -172,7 +173,7 @@ class {{=objectWrapper}} {
       {{?}}
       {{~}}
     }
-    this.serialize();
+    this.freeze();
   }
 
   static createFromRefObject(refObject) {
@@ -198,30 +199,64 @@ class {{=objectWrapper}} {
   }
 
   toRawROS() {
+    this.freeze(true);
     return this._refObject.ref();
   }
 
-  serialize() {
+  freeze(own = false) {
     {{~ it.spec.fields :field}}
     {{? !field.type.isPrimitiveType || field.type.isArray}}
-    this._wrapperFields.{{=field.name}}.serialize();
+    this._wrapperFields.{{=field.name}}.freeze(own);
     this._refObject.{{=field.name}} = this._wrapperFields.{{=field.name}}.refObject;
     {{?? field.type.type === 'string' && it.spec.msgName !== 'String'}}
+    if (own) {
+      primitiveTypes.initString(this._wrapperFields.{{=field.name}}.refObject.ref(), own);
+    }
     this._refObject.{{=field.name}} = this._wrapperFields.{{=field.name}}.refObject;
+    {{?? it.spec.msgName === 'String'}}
+    if (own) {
+      primitiveTypes.initString(this._refObject.ref(), own);
+    }
     {{?}}
     {{~}}
   }
 
-  deserialize() {
+  serialize() {
+    this.freeze();
+    return this._refObject.ref();
+  }
+
+  deserialize(refObject) {
     {{~ it.spec.fields :field}}
     {{? !field.type.isPrimitiveType || field.type.isArray}}
-    this._wrapperFields.{{=field.name}}.copyRefObject(this._refObject.{{=field.name}});
+    this._wrapperFields.{{=field.name}}.copyRefObject(refObject.{{=field.name}});
     {{?? field.type.type === 'string' && it.spec.msgName !== 'String'}}
-    this._wrapperFields.{{=field.name}}.data = this._refObject.{{=field.name}}.data;
+    this._wrapperFields.{{=field.name}}.data = refObject.{{=field.name}}.data;
+    {{?? field.type.isPrimitiveType}}
+    this._refObject.{{=field.name}} = refObject.{{=field.name}};
+    {{?}}
+    {{~}}
+  }
+
+  static freeStruct(refObject) {
+    {{~ it.spec.fields :field}}
+    {{? field.type.isArray}}
+    if (refObject.{{=field.name}}.size != 0) {
+      {{=getWrapperNameByType(field.type)}}.ArrayType.freeArray(refObject.{{=field.name}});
+      deallocator.freeStructMember(refObject.{{=field.name}}, {{=getWrapperNameByType(field.type)}}.refObjectArrayType, 'data');
+    }
+    {{?? !field.type.isPrimitiveType || (field.type.type === 'string' && it.spec.msgName !== 'String')}}
+    {{=getWrapperNameByType(field.type)}}.freeStruct(refObject.{{=field.name}});
+    {{?? it.spec.msgName === 'String'}}
+    deallocator.freeStructMember(refObject, {{=getWrapperNameByType(field.type)}}.refObjectType, '{{=field.name}}');
     {{?}}
     {{~}}
   }
 
+  static destoryRawROS(msg) {
+    {{=objectWrapper}}.freeStruct(msg.refObject);
+  }
+
   static type() {
     return {pkgName: '{{=it.messageInfo.pkgName}}', subFolder: '{{=it.messageInfo.subFolder}}', interfaceName: '{{=it.messageInfo.interfaceName}}'};
   }
@@ -319,20 +354,16 @@ class {{=arrayWrapper}} {
     {{?}}
   }
 
-  serialize() {
+  freeze(own) {
     this._wrappers.forEach((wrapper, index) => {
-      wrapper.serialize();
+      wrapper.freeze(own);
       this._refArray[index] = wrapper.refObject;
     });
     this._refObject.size = this._wrappers.length;
     this._refObject.capacity = this._wrappers.length;
     this._refObject.data = this._refArray.buffer;
   }
 
-  deserialize(refObject) {
-    copyRefObject(refObject)
-  }
-
   get refObject() {
     return this._refObject;
   }
@@ -406,6 +437,14 @@ class {{=arrayWrapper}} {
       this._wrappers[index].copy(wrapper);
     });
   }
+
+  static freeArray(refObject) {
+    let refObjectArray = refObject.data;
+    refObjectArray.length = refObject.size;
+    for (let index = 0; index < refObject.size; index++) {
+      {{=objectWrapper}}.freeStruct(refObjectArray[index]);
+    }
+  }
 }
 
 {{? it.spec.constants != undefined && it.spec.constants.length}}
diff --git a/src/executor.cpp b/src/executor.cpp
@@ -24,7 +24,6 @@
 namespace rclnodejs {
 
 static std::exception_ptr g_exception_ptr = nullptr;
-static bool g_handle_closed = false;
 
 Executor::Executor(HandleManager* handle_manager, Delegate* delegate)
     : async_(nullptr), handle_manager_(handle_manager), delegate_(delegate) {
@@ -39,7 +38,6 @@ void Executor::Start() {
   if (!running_.load()) {
     async_ = new uv_async_t();
     uv_async_init(uv_default_loop(), async_, DoWork);
-    g_handle_closed = false;
     async_->data = this;
 
     // Mark flag before creating thread
@@ -55,18 +53,18 @@ void Executor::Stop() {
     // Make sure async_ is not used anymore
     running_.store(false);
     uv_thread_join(&thread_);
+    static bool handle_closed = false;
 
     if (uv_is_active(reinterpret_cast<uv_handle_t*>(async_))) {
       uv_close(reinterpret_cast<uv_handle_t*>(async_),
                [](uv_handle_t* async) -> void {
                  // Important Notice:
                  //  This might be called after Executor::~Executor()
                  //  Don't free Executor::async_ in Executor's dtor
-                 //
                  free(async);
-                 g_handle_closed = true;
+                 handle_closed = true;
                });
-      while (!g_handle_closed)
+      while (!handle_closed)
         uv_run(uv_default_loop(), UV_RUN_ONCE);
     }
   }
diff --git a/src/rcl_bindings.cpp b/src/rcl_bindings.cpp
@@ -158,7 +158,6 @@ NAN_METHOD(RclTake) {
   rcl_subscription_t* subscription =
       reinterpret_cast<rcl_subscription_t*>(subscription_handle->ptr());
   void* msg_taken = node::Buffer::Data(info[1]->ToObject());
-
   rcl_ret_t ret = rcl_take(subscription, msg_taken, nullptr);
 
   if (ret != RCL_RET_OK && ret != RCL_RET_SUBSCRIPTION_TAKE_FAILED) {
@@ -673,6 +672,34 @@ NAN_METHOD(Shutdown) {
   info.GetReturnValue().Set(Nan::Undefined());
 }
 
+NAN_METHOD(InitString) {
+    void* buffer = node::Buffer::Data(info[0]->ToObject());
+    rosidl_generator_c__String* ptr =
+        reinterpret_cast<rosidl_generator_c__String*>(buffer);
+
+    rosidl_generator_c__String__init(ptr);
+    info.GetReturnValue().Set(Nan::Undefined());
+}
+
+NAN_METHOD(FreeMemeoryAtOffset) {
+  v8::Local<v8::Value> buf = info[0];
+  if (!node::Buffer::HasInstance(buf)) {
+    return Nan::ThrowTypeError("Buffer instance expected as first argument");
+  }
+
+  int64_t offset =
+      info[1]->IsNumber() ? Nan::To<int64_t>(info[1]).FromJust() : 0;
+  char* ptr = node::Buffer::Data(buf.As<v8::Object>()) + offset;
+
+  if (ptr == nullptr) {
+    return Nan::ThrowError("Cannot read from NULL pointer");
+  }
+
+  char* val = *reinterpret_cast<char**>(ptr);
+  free(val);
+  info.GetReturnValue().Set(Nan::Undefined());
+}
+
 uint32_t GetBindingMethodsCount(BindingMethod* methods) {
   uint32_t count = 0;
   while (methods[count].function) {
@@ -710,6 +737,8 @@ BindingMethod binding_methods[] = {
     {"expandTopicName", ExpandTopicName},
     {"getNodeName", GetNodeName},
     {"getNamespace", GetNamespace},
+    {"initString", InitString},
+    {"freeMemeoryAtOffset", FreeMemeoryAtOffset},
     {"", nullptr}};
 
 }  // namespace rclnodejs
diff --git a/test/blacklist.json b/test/blacklist.json

Original file line number	Diff line number	Diff line change
`@@ -34,6 +34,7 @@`
`34`	`34`	`'-lrcl',`
`35`	`35`	`'-lrcutils',`
`36`	`36`	`'-lrmw',`
	`37`	`+ '-lrosidl_generator_c',`
`37`	`38`	`],`
`38`	`39`	`'conditions': [`
`39`	`40`	`[`
Original file line number	Diff line number	Diff line change
`@@ -57,22 +57,17 @@ class Client extends Entity {`
`57`	`57`	`throw new TypeError('Invalid argument');`
`58`	`58`	`}`
`59`	`59`
`60`		`- rclRequest.serialize();`
`61`		`- let rawROSRequest = rclRequest.toRawROS();`
	`60`	`+ let rawROSRequest = rclRequest.serialize();`
`62`	`61`	`this._sequenceNumber = rclnodejs.sendRequest(this._handle, rawROSRequest);`
`63`	`62`	`this._callback = callback;`
`64`	`63`	`}`
`65`	`64`
`66`		`- get takenResponse() {`
`67`		`- return this._response.toRawROS();`
`68`		`- }`
`69`		`-`
`70`	`65`	`get sequenceNumber() {`
`71`	`66`	`return this._sequenceNumber;`
`72`	`67`	`}`
`73`	`68`
`74`		`- processResponse() {`
`75`		`- this._response.deserialize();`
	`69`	`+ processResponse(response) {`
	`70`	`+ this._response.deserialize(response);`
`76`	`71`	`this._callback(this._response);`
`77`	`72`	`}`
`78`	`73`
Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,10 @@ class Entity {`
`33`	`33`	`get qos() {`
`34`	`34`	`return this._qos;`
`35`	`35`	`}`
	`36`	`+`
	`37`	`+ get typeClass() {`
	`38`	`+ return this._typeClass;`
	`39`	`+ }`
`36`	`40`	`}`
`37`	`41`
`38`	`42`	`module.exports = Entity;`
Original file line number	Diff line number	Diff line change
`@@ -50,8 +50,7 @@ class Publisher extends Entity {`
`50`	`50`	`rclMessage.data = message;`
`51`	`51`	`}`
`52`	`52`
`53`		`- rclMessage.serialize();`
`54`		`- let rawRosMessage = rclMessage.toRawROS();`
	`53`	`+ let rawRosMessage = rclMessage.serialize();`
`55`	`54`	`if (rawRosMessage) {`
`56`	`55`	`rclnodejs.publish(this._handle, rawRosMessage);`
`57`	`56`	`} else {`
Original file line number	Diff line number	Diff line change
`@@ -30,12 +30,8 @@ class Subscription extends Entity {`
`30`	`30`	`this._message = new typeClass();`
`31`	`31`	`}`
`32`	`32`
`33`		`- get takenMsg() {`
`34`		`- return this._message.toRawROS();`
`35`		`- }`
`36`		`-`
`37`		`- processResponse() {`
`38`		`- this._message.deserialize();`
	`33`	`+ processResponse(msg) {`
	`34`	`+ this._message.deserialize(msg);`
`39`	`35`	`this._callback(this._message);`
`40`	`36`	`}`
`41`	`37`