lldpctl_watch leads to hung on exit

[vlaggeor]

Writing a simple application that has

lldpctl_watch_callback(conn, watchcb, NULL)
...
lldpctl_watch(conn)
...
lldpctl_release(conn);

will lead to a hung.

This is because because lldpctl_watch remains in a while loop on watch_triggered that will be called only. when change is received and notification is called in check_for_notification

int
lldpctl_watch(lldpctl_conn_t *conn)
{
    int rc = 0;

    RESET_ERROR(conn);

    if (conn->state != CONN_STATE_WATCHING)
        return SET_ERROR(conn, LLDPCTL_ERR_INVALID_STATE);

    conn->watch_triggered = 0;
    while (!conn->watch_triggered) {
        rc = _lldpctl_needs(conn, 1);                            <-------
        if (rc < 0) return SET_ERROR(conn, rc);
    }

    RESET_ERROR(conn);
    return 0;
}

ssize_t
lldpctl_recv(lldpctl_conn_t *conn, const uint8_t *data, size_t length)
{

.....

    /* Read all notifications */
    while(!check_for_notification(conn));        <----------

    RESET_ERROR(conn);

    return conn->input_buffer_len;
}

static int
check_for_notification(lldpctl_conn_t *conn)
{
....
    rc = ctl_msg_recv_unserialized(&conn->input_buffer,
        &conn->input_buffer_len,
        NOTIFICATION,
        &p,
        &MARSHAL_INFO(lldpd_neighbor_change));
    if (rc != 0) return rc;              <----------
    change = p;

    /* We have a notification, call the callback */
    if (conn->watch_cb) {
....
        conn->watch_triggered = 1;       <---------- 
        goto end;
    }

[vlaggeor]

A proposed fix is in https://github.com/lldpd/lldpd/pull/676

[rbu9fe]

I face the same issue now. Since the lldpctl_watch function is read()-blocked it must somehow be unblocked. I don't see how this is achieved by #676.

I guess whether to unblock implicitly when closing the connection or to add a corresponding public function is just a design flavor. In any case this must be triggered by a separate thread.

@vincentbernat Any ideas how to unblock the read()-blocked thread? Perhaps by a sequence similar to:

1. Invalidate the connection (e.g. by setting the state to something invalid).
2. Send a signal to make read unblock with EINTR.
3. Before continuing to read again check for a valid state and return if it was invalidated.

[vincentbernat]

I think the connection should just be closed. A signal is not pretty and subject to some race condition (if the thread unblock before processing the signal for example).