Some progress.Send calls not making it to nodejs land

Question

I've made a Node addon using AsyncProgressWorker thread to handle my socket messages. Here is my code:

class ProgressWorker : public AsyncProgressWorker {
 public:
  ProgressWorker(
      Callback *callback
    , Callback *progress)
    : AsyncProgressWorker(callback), progress(progress) {}
  ~ProgressWorker() {}

  void Execute (const AsyncProgressWorker::ExecutionProgress& progress) {
    char response[4096];
    int result;
    int connected = 1;
    int timeout = 0;
    int pending = 0;

    while(connected) {
        result = sctp_recvmsg(sock, (void *)&response, (size_t)sizeof(response), NULL, 0, 0, 0);
        if (result > 0 && result < 4095) {
            if (debug) {
                printf("Server replied (size %d)\n", result);
            }
            pending = 0;
            progress.Send((const char *)response, size_t(result));
            result = 0;
        }
        else {
            // Don't mind my timeout mechanism. :))
            if ((result == -1 && errno != EWOULDBLOCK) || pending) {
                if (timeout == 0) {
                    printf("Can't receive from other end. Waiting for 3 seconds. Error code: %d\n", errno);
                    pending = 1;
                }
                if (timeout >= 3000) {
                    connected = 0;
                    close(sock);
                }
                else {
                    timeout += 5;
                    usleep(5000);
                }
            }
            else {
                usleep(5000);
            }
        }
    }

  }

  void HandleProgressCallback(const char *data, size_t count) {
    HandleScope scope;

    v8::Local<v8::Value> argv[] = {
        CopyBuffer(const_cast<char*>(data), count).ToLocalChecked()
    };
    progress->Call(1, argv); // This is the callback to nodejs
  }

 private:
  Callback *progress;
};

Now I haven't stress-tested this until tonight then I noticed that some messages won't make it back to node. It will print my "Server replied" debug log but won't log my debug logs I put on the progress callback. Am I missing something here? Thanks in advance.

why not implement ACK? (just for making sure when packets drop) — EMX, Sep 04 '17 at 07:06
@EMX Sorry for the answer, but I'm not sure about ACK. All I can say is this is an SCTP socket implementation. I'm just relying on the sctp_recvmsg return value. The missing messages passed my debug log, just the progress callback no being called. Thanks. — Phenelo, Sep 04 '17 at 07:12
Shouldn't you pass pointer to `::v8::Isolate` into `HandleScope` constructor? — user7860670, Sep 04 '17 at 08:54
note: the class in the OP has a memory leak. It's not your fault - this same memory leak used to exist in the _NAN_ unit tests as well, until recently. See here for more info: https://github.com/nodejs/nan/issues/698 To fix this, just `delete progress;` in `~ProgressWorker()`, like so: ``` ~ProgressWorker() { delete progress; } ``` — mkrufky, Oct 08 '17 at 18:06
I see. That's why when I monitored, there was a slow increase in the heap usage. Although I've patched up the addon, still it exists. Will try to update with these and see how it goes. Thanks! — Phenelo, Oct 09 '17 at 06:16

mkrufky · Accepted Answer · 2017-11-17T11:55:11.183

AsyncProgressWorker is based on a uv_async_t, which allows any thread to wake the main thread. However, as stated in the documentation:

libuv will coalesce calls to uv_async_send(), that is, not every call to it will yield an execution of the callback. For example: if uv_async_send() is called 5 times in a row before the callback is called, the callback will only be called once. If uv_async_send() is called again after the callback was called, it will be called again.

^^ This is the reason that you may sometimes not receive some events while your application is under stress. Above this line is the answer to the question. Below is my "above and beyond" possible solution to deal with your problem:

It so happens that I am working on adding a new alternative to AsyncProgressWorker that promises to deliver every event, just as AsyncProgressWorker does, but using a queue. This feature was recently merged into NAN. If you want to test it, try out the git repository at https://github.com/nodejs/nan , and then replace your AsyncProgressWorker with AsyncProgressQueueWorker<char> Re-run your tests and all events will be delivered.

The pull request to add this new feature is here: https://github.com/nodejs/nan/pull/692 - merged on Oct 6, 2017.

This new feature was released in NAN version 2.8.0

You can use this new class template by altering your package.json to use nan version 2.8.0 or later:

   "dependencies": {
     "nan": "^2.8.0"
   },

Sorry for the late accept of your answer, as I was searching for the solution I forgot about my question here. Anyways, it was recently that I found out that that was the case why the messages were not going through. In fact, I'm on the process of creating my own queueing of messages. But I'll try this out. Thanks. — Phenelo, Sep 11 '17 at 03:20
Thank you, it works! My remaining problem now is to queue the part of sending of messages. But that's another story. — Phenelo, Sep 11 '17 at 06:51
I have altered my open pull request to rename the template class from `AsyncWakeRequestor` to `AsyncProgressQueueWorker` - be sure to update your code accordingly. — mkrufky, Sep 18 '17 at 15:15

Some progress.Send calls not making it to nodejs land

1 Answers1