Skip to content

Commit bf25db3

Browse files
Roytakmichalvasko
authored andcommitted
session UPDATE wait for ssh channel close
Wait until we receiver SSH EOF from the peer, which avoids needlessly printing socket exception callbacks from libssh. 100ms should be more than enough to receive this message, if not just free as before. Also possibly fix some race conditions by locking io_lock before sending the EOF. Fixes #589
1 parent 37286fb commit bf25db3

1 file changed

Lines changed: 24 additions & 5 deletions

File tree

src/session.c

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -796,18 +796,37 @@ nc_session_free_transport(struct nc_session *session, int *multisession)
796796
#ifdef NC_ENABLED_SSH_TLS
797797
case NC_TI_SSH: {
798798
int r;
799+
struct timespec ts;
799800

800-
if (connected) {
801-
ssh_channel_send_eof(session->ti.libssh.channel);
802-
ssh_channel_free(session->ti.libssh.channel);
803-
}
804801
/* There can be multiple NETCONF sessions on the same SSH session (NETCONF session maps to
805802
* SSH channel). So destroy the SSH session only if there is no other NETCONF session using
806803
* it. Also, avoid concurrent free by multiple threads of sessions that share the SSH session.
807804
*/
808805
/* SESSION IO LOCK */
809806
r = nc_mutex_lock(session->io_lock, NC_SESSION_FREE_LOCK_TIMEOUT, __func__);
810807

808+
if (connected) {
809+
/* send EOF to the peer, but do not close the channel yet, wait for the peer to send EOF too.
810+
* 100ms timeout should be enough for the peer to react and send EOF,
811+
* if not, just continue with freeing the session and closing the channel.
812+
* This is done to avoid libssh WRN log about reading from a closed channel */
813+
ssh_channel_send_eof(session->ti.libssh.channel);
814+
nc_timeouttime_get(&ts, 100);
815+
while (!ssh_channel_is_eof(session->ti.libssh.channel)) {
816+
/* poll for the EOF, non-blocking */
817+
if (ssh_channel_poll(session->ti.libssh.channel, 0) == SSH_ERROR) {
818+
/* if poll fails, just break and continue with freeing the session, it will be closed anyway */
819+
break;
820+
}
821+
if (nc_timeouttime_cur_diff(&ts) < 1) {
822+
/* waited long enough, continue with freeing */
823+
break;
824+
}
825+
usleep(NC_TIMEOUT_STEP);
826+
}
827+
ssh_channel_free(session->ti.libssh.channel);
828+
}
829+
811830
if (session->ti.libssh.next) {
812831
for (siter = session->ti.libssh.next; siter != session; siter = siter->ti.libssh.next) {
813832
if (siter->status != NC_STATUS_STARTING) {
@@ -869,7 +888,7 @@ nc_session_free_transport(struct nc_session *session, int *multisession)
869888
sock = nc_tls_get_fd_wrap(session);
870889

871890
if (connected) {
872-
/* notify the peer that we're shutting down */
891+
/* notify the peer that we're shutting down, we don't need to wait for the peer's response */
873892
nc_tls_close_notify_wrap(session->ti.tls.session);
874893
}
875894

0 commit comments

Comments
 (0)