SAP / node-rfc

Asynchronous, non-blocking SAP NW RFC SDK bindings for Node.js
Apache License 2.0
251 stars 73 forks source link

RFC_INVALID_HANDLE / Segmentation Fault: 11 #99

Closed PaulWieland closed 4 years ago

PaulWieland commented 5 years ago

I'm having an issue where when I create many clients in a loop using cwait, several of the iterations will fail with

call error: { name: 'RfcLibError',
  code: 13,
  key: 'RFC_INVALID_HANDLE',
  message: 'An invalid handle was passed to the API call' }

And sometimes nodejs will crash with a Segmentation Fault: 11.

I am using the 7.50 release of the nwrfcsdk and the @next branch of node-rfc. The issue happens on both Linux (node running in docker) and in native OSX Mojave (10.14.3)

Here is the program:

var rfcClient = require('node-rfc').Client;
var cwait = require('cwait');

// Read a table definition
function doRFC(){

    return new Promise(function(resolve,reject){
        var client = new rfcClient({
            user: 'SOMEUSERNAME',
            passwd: "SUPERSECRET",
            ashost: "10.0.0.0",
            sysnr: "00",
            client: "100",
            lang: "EN",
        });

        client.open().then(() => {
            client.call(
                "RFC_READ_TABLE",
                {
                    QUERY_TABLE: "MARA",
                    NO_DATA: "X"
                }
            ).then((res) => {
                console.log("result");
                client.close();
                return resolve();
            }).catch((err) => {
                console.log("call error:",err);
                client.close();
                return reject(err);
            });

        }).catch((err) => {
            client.close();
            console.log("open Error:",err);
            return reject(err);
        });
    });
}

// Create a queue for processing the async calls 10 at a time
var queue = new cwait.TaskQueue(Promise, 10);

// Submit the RFC n times to the cwait queue
for(var i = 0; i<200; i++){
    // doRFC();
    queue.wrap(doRFC)();
};
PaulWieland commented 5 years ago

Update: Same issue using the more popular async library:

var async = require("async");
var rfcClient = require('node-rfc').Client;

// create a queue object with concurrency 10
var q = async.queue(function(task,callback){

    var client = new rfcClient({
            user: 'SOMEUSERNAME',
            passwd: "SUPERSECRET",
            ashost: "10.0.0.0",
            sysnr: "00",
            client: "100",
            lang: "EN",
    });

    client.open().then(() => {

        client.call(
            "RFC_READ_TABLE",
            {
                QUERY_TABLE: "MARA",
                NO_DATA: "X"
            }
        ).then((res) => {
            console.log("result "+task.count);
            client.close();

            callback();

        }).catch((err) => {
            console.log("call error "+task.count,err);
            client.close();

            callback();
        });

    }).catch((err) => {
        client.close();
        console.log("open Error:"+task.count,err);

        callback();
    });

}, 10);

// assign a callback
q.drain(function() {
    console.log('all items have been processed');
});

// assign an error callback
q.error(function(err, task) {
    console.error('task experienced an error',err,task);
});

for(var i = 0; i<300; i++){
    // doRFC();
    q.push({count: i});
};
PaulWieland commented 5 years ago

I suspect something is wrong with my implementation of promises (which is entirely possible since I just started using them for this project).

I just ported the code to use async with node-rfc's callbacks instead of promises and I do not get any segmentation faults or RfcLibErrors.

EDIT: never mind, it’s segfaulting with callbacks too.

bsrdjan commented 5 years ago

Segfaults should not happen, need to check that in detail and testing the async example in my Darwin and Linux systems, I got RFC_INVALID_HANDLE RfcLibErrors too.

I did not have a time to check the async library in detail but the RFC_INVALID_HANDLE is usually caused by sharing the same node-rfc client connection instance among more than one thread.

As described in this comment, the nodejs application should ensure it does not happening. Concurrency unit tests give examples working on all platforms. When adding more libraries in top, it is fine to create multiple threads (nodejs itself is a single thread but underlying OS is multi threaded) with client connection instances in each thread. The mechanism must be provided however that client instances are not shared among threads.

PaulWieland commented 5 years ago

I tested out the connection pool instead of client.open and this seems stable. The only issue I have now is figuring out how to detect when the connection is severed and to re-connect the pool.

Is there a property/method I'm not seeing that I can use to check if the connection is alive?

It seems that when using pool.acquire(), the connection will not be re-established if the link was severed - you have to instantiate a new Pool.

bsrdjan commented 5 years ago

The client.isAlive boolean property should help (examples in unit tests).

I will check the case with pool.acquire()after connection broken.

PaulWieland commented 5 years ago

I did not have a time to check the async library in detail but the RFC_INVALID_HANDLE is usually caused by sharing the same node-rfc client connection instance among more than one thread. As described in this comment, the nodejs application should ensure it does not happening.

I suspect that's exactly what was happening in my sample above, but I do not understand why. I wrapped the entire client instantiation, open, call and close inside it's own anonymous function. I do not understand how another thread could have tried to use the same client instance.

PaulWieland commented 4 years ago

So after switching everything to use the connection pool, I haven't had any more segmentation faults. I'll close the issue to keep the queue clean, but for any one else having a similar problem, try and use the connection pool instead.