thrift-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Markus Hocke (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (THRIFT-4424) Flush in TWebSocketTransport pushes callbacks twice if transport is open
Date Wed, 13 Dec 2017 16:05:00 GMT

     [ https://issues.apache.org/jira/browse/THRIFT-4424?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Markus Hocke updated THRIFT-4424:
---------------------------------
    Description: 
The flush code looks like this (see also comments in this snippet):

{code:javascript}
   flush: function(async, callback) {
      var self = this;
      if (this.isOpen()) {
        //Send data and register a callback to invoke the client callback
        this.socket.send(this.send_buf);
        this.callbacks.push((function() {
          var clientCallback = callback;
          return function(msg) {
            self.setRecvBuffer(msg);
            clientCallback();
          };
        }()));
        // Here the callback gets pushed a second time.
        if(callback) { // What is the intention of this code section?
          this.callbacks.push((function() {
            var clientCallback = callback;
            return function(msg) {
              self.setRecvBuffer(msg);
              clientCallback();
            };
          }()));
        }
      } else {
        //Queue the send to go out __onOpen
        this.send_pending.push({
          buf: this.send_buf,
          cb: callback
        });
      }
    }
{code}


We're having trouble with callbacks called twice in our web client implementation that assumes
that the callback is only called once. This should be a default behaviour in my opinion. I'd
suggest to remove this if-clause.

  was:
The flush code looks like this (see also comments in this snippet):
{{    flush: function(async, callback) {
      var self = this;
      if (this.isOpen()) {
        //Send data and register a callback to invoke the client callback
        this.socket.send(this.send_buf);
        this.callbacks.push((function() {
          var clientCallback = callback;
          return function(msg) {
            self.setRecvBuffer(msg);
            clientCallback();
          };
        }()));
        // Here the callback gets pushed a second time.
        if(callback) { // What is the intention of this code section?
          this.callbacks.push((function() {
            var clientCallback = callback;
            return function(msg) {
              self.setRecvBuffer(msg);
              clientCallback();
            };
          }()));
        }
      } else {
        //Queue the send to go out __onOpen
        this.send_pending.push({
          buf: this.send_buf,
          cb: callback
        });
      }
    },}}

We're having trouble with callbacks called twice in our web client implementation that assumes
that the callback is only called once. This should be a default behaviour in my opinion. I'd
suggest to remove this if-clause.


> Flush in TWebSocketTransport pushes callbacks twice if transport is open
> ------------------------------------------------------------------------
>
>                 Key: THRIFT-4424
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4424
>             Project: Thrift
>          Issue Type: Bug
>          Components: JavaScript - Library
>    Affects Versions: 0.11.0
>            Reporter: Markus Hocke
>
> The flush code looks like this (see also comments in this snippet):
> {code:javascript}
>    flush: function(async, callback) {
>       var self = this;
>       if (this.isOpen()) {
>         //Send data and register a callback to invoke the client callback
>         this.socket.send(this.send_buf);
>         this.callbacks.push((function() {
>           var clientCallback = callback;
>           return function(msg) {
>             self.setRecvBuffer(msg);
>             clientCallback();
>           };
>         }()));
>         // Here the callback gets pushed a second time.
>         if(callback) { // What is the intention of this code section?
>           this.callbacks.push((function() {
>             var clientCallback = callback;
>             return function(msg) {
>               self.setRecvBuffer(msg);
>               clientCallback();
>             };
>           }()));
>         }
>       } else {
>         //Queue the send to go out __onOpen
>         this.send_pending.push({
>           buf: this.send_buf,
>           cb: callback
>         });
>       }
>     }
> {code}
> We're having trouble with callbacks called twice in our web client implementation that
assumes that the callback is only called once. This should be a default behaviour in my opinion.
I'd suggest to remove this if-clause.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Mime
View raw message