You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by jk...@apache.org on 2018/07/17 16:19:55 UTC

[thrift] branch master updated: THRIFT-3950: Memory leak while calling oneway methods (#1568)

This is an automated email from the ASF dual-hosted git repository.

jking pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/thrift.git


The following commit(s) were added to refs/heads/master by this push:
     new f2867c2  THRIFT-3950: Memory leak while calling oneway methods (#1568)
f2867c2 is described below

commit f2867c24984aa53edec54a138c03db934221bdea
Author: bforbis <bp...@gmail.com>
AuthorDate: Tue Jul 17 12:19:49 2018 -0400

    THRIFT-3950: Memory leak while calling oneway methods (#1568)
    
    * THRIFT-3950: Memory leak while calling oneway methods
    * THRIFT-3950: Update NodeJS Oneway tests
---
 compiler/cpp/src/thrift/generate/t_js_generator.cc | 9 ++++++++-
 lib/nodejs/test/test_driver.js                     | 9 +++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/compiler/cpp/src/thrift/generate/t_js_generator.cc b/compiler/cpp/src/thrift/generate/t_js_generator.cc
index 57baa9b..4d1cea6 100644
--- a/compiler/cpp/src/thrift/generate/t_js_generator.cc
+++ b/compiler/cpp/src/thrift/generate/t_js_generator.cc
@@ -1565,7 +1565,14 @@ void t_js_generator::generate_service_client(t_service* tservice) {
                << ".writeMessageEnd();" << endl;
 
     if (gen_node_) {
-      f_service_ << indent() << "return this.output.flush();" << endl;
+      if((*f_iter)->is_oneway()) {
+        f_service_ << indent() << "this.output.flush();" << endl;
+        f_service_ << indent() << "var callback = this._reqs[this.seqid()] || function() {};" << endl;
+        f_service_ << indent() << "delete this._reqs[this.seqid()];" << endl;
+        f_service_ << indent() << "callback(null);" << endl;
+      } else {
+        f_service_ << indent() << "return this.output.flush();" << endl;
+      }
     } else {
       if (gen_jquery_) {
         f_service_ << indent() << "return this.output.getTransport().flush(callback);" << endl;
diff --git a/lib/nodejs/test/test_driver.js b/lib/nodejs/test/test_driver.js
index 03ec513..4612a32 100644
--- a/lib/nodejs/test/test_driver.js
+++ b/lib/nodejs/test/test_driver.js
@@ -125,7 +125,8 @@ exports.ThriftTestDriver = function(client, callback) {
     });
 
     client.testOneway(0, function(err, response) {
-      assert.fail('testOneway should not answer');
+      assert.error(err, 'testOneway: no callback error');
+      assert.strictEqual(response, undefined, 'testOneway: void response');
     });
 
     checkOffByOne(function(done) {
@@ -223,7 +224,11 @@ exports.ThriftTestDriverPromise = function(client, callback) {
       })
       .fail(fail('testException'));
 
-    client.testOneway(0, fail('testOneway: should not answer'));
+    client.testOneway(0)
+      .then(function(response) {
+        assert.strictEqual(response, undefined, 'testOneway: void response')
+      })
+      .fail(fail('testOneway: should not reject'));
 
     checkOffByOne(function(done) {
       client.testI32(-1)