Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-4424

Flush in TWebSocketTransport pushes callbacks twice if transport is open

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Open
    • Major
    • Resolution: Unresolved
    • 0.11.0
    • None
    • JavaScript - Library
    • None

    Description

      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.

      Attachments

        Activity

          People

            Unassigned Unassigned
            mhc Markus Hocke
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated: