libnfs icon indicating copy to clipboard operation
libnfs copied to clipboard

connect callback isn't called on rpc_disconnect()

Open crusader-mike opened this issue 6 years ago • 5 comments

Following snippet fails to call connect_cb() and leaks related data structure:

static void connect_cb(rpc_context* rpc, int status, void* command_data, void *private_data)
{
}

...

    rpc_context* rpc = rpc_init_context();

    int err = rpc_connect_program_async(rpc, "server-address", 
                           MOUNT_PROGRAM, MOUNT_V3, &connect_cb, nullptr);

    rpc_disconnect(rpc, "AAAA");

    rpc_destroy_context(rpc);

basically, if program decides to "wrap it up" while connection is being established -- callback isn't called (this may cause issues) and we leak underlying callback structure.

crusader-mike avatar Dec 02 '19 22:12 crusader-mike

If rpc_connect_program_async() returns an error then the callback will not be invoked. That is true for all _async() functions.

Or did I misunderstand your report?

sahlberg avatar Dec 16 '19 03:12 sahlberg

<clicked wrong button> reopened

sahlberg avatar Dec 16 '19 03:12 sahlberg

rpc_disconnect goes over all outstanding calls and invokes their callbacks with some error code (like canceled). Unfortunately it doesn't invoke callback passed into rpc_connect_* if rpc context is in "connecting" state. As result related libnfs structure leaks (plus whatever structures user cb was supposed to free will leak too).

crusader-mike avatar Dec 16 '19 17:12 crusader-mike

I see, try this latest patch for master. It should do what you want I think.

On Tue, Dec 17, 2019 at 3:41 AM crusader-mike [email protected] wrote:

rpc_disconnect goes over all outstanding calls and invokes their callbacks with some error code (like canceled). Unfortunately it doesn't invoke callback passed into rpc_connect_* if rpc context state is in "connecting" state. As result related libnfs structure leaks (plus whatever structures user cb was supposed to free will leak too).

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/sahlberg/libnfs/issues/303?email_source=notifications&email_token=AADY3EDYL5W3JZAZJC67ULDQY64TZA5CNFSM4JULIA22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG7P75I#issuecomment-566165493, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADY3EG6GVMK6FNFWN3TCGTQY64TZANCNFSM4JULIA2Q .

sahlberg avatar Dec 16 '19 22:12 sahlberg

Looks ok. My only concern is rpc->error_string being passed into callback (it can be NULL). But (I guess) NULL error string passed into callback is ok.

crusader-mike avatar Dec 17 '19 00:12 crusader-mike