stephane / libmodbus

A Modbus library for Linux, Mac OS, FreeBSD and Windows
http://libmodbus.org
GNU Lesser General Public License v2.1
3.33k stars 1.71k forks source link

Potential lock-up in send_msg() #199

Closed spachner closed 10 years ago

spachner commented 10 years ago

Hi,

in modbus.c::send_msg() a potential lock-up exists. Assume error_recovery is active and ctx->backend->send() will never succeed.

Propose a max try count and exit.

do {
    rc = ctx->backend->send(ctx, msg, msg_length);
    if (rc == -1) {
        _error_print(ctx, NULL);
        if (ctx->error_recovery & MODBUS_ERROR_RECOVERY_LINK) {
            int saved_errno = errno;

            if ((errno == EBADF || errno == ECONNRESET || errno == EPIPE)) {
                modbus_close(ctx);
                modbus_connect(ctx);
            } else {
                _sleep_and_flush(ctx);
            }
            errno = saved_errno;
        }
    }
} while ((ctx->error_recovery & MODBUS_ERROR_RECOVERY_LINK) && rc == -1);
stephane commented 10 years ago

Why you say send() call will never succeed? There is already an issue to talk about creation of an error counter (#40).

spachner commented 10 years ago

Hi Stephane,

sorry I did not see #40 which is quite old. But in my source fetched from http://libmodbus.org/ 3.0.5 stable the function look like this.

/* Sends a request/response / static int send_msg(modbus_t ctx, uint8_t *msg, int msg_length) { int rc; int i;

msg_length = ctx->backend->send_msg_pre(msg, msg_length);

if (ctx->debug) {
    for (i = 0; i < msg_length; i++)
        printf("[%.2X]", msg[i]);
    printf("\n");
}

/* In recovery mode, the write command will be issued until to be
   successful! Disabled by default. */
do {
    rc = ctx->backend->send(ctx, msg, msg_length);
    if (rc == -1) {
        _error_print(ctx, NULL);
        if (ctx->error_recovery & MODBUS_ERROR_RECOVERY_LINK) {
            int saved_errno = errno;

            if ((errno == EBADF || errno == ECONNRESET || errno == EPIPE)) {
                modbus_close(ctx);
                modbus_connect(ctx);
            } else {
                _sleep_and_flush(ctx);
            }
            errno = saved_errno;
        }
    }
} while ((ctx->error_recovery & MODBUS_ERROR_RECOVERY_LINK) &&
         rc == -1);

if (rc > 0 && rc != msg_length) {
    errno = EMBBADDATA;
    return -1;
}

return rc;

}

Looks different than discussed in #40 which deals with send_attempts++ < MODBUS_MAX_SEND_RETRIES);

Stefan

On 05.03.2014, at 16:01, Stéphane Raimbault notifications@github.com wrote:

Why you say send() call will never succeed? There is already an issue to talk about creation of an error counter (#40).

— Reply to this email directly or view it on GitHub.