ssh callhome occasionally does not exit session about libnetconf2 HOT 8 CLOSED

Hi,
if I understand it correctly, the appended patch could fix it, could you please try it?

Regards,
Michal

ch_sess_free.patch.txt

from libnetconf2.

This would not work since pthread_cond_timedwait(session->opts.server.ch_cond, ...) does not always exit immediately when the associated signal is called (in nc_session_free), thus allowing the destroy to happen while still in pthread_cond_timedwait(session->opts.server.ch_cond, ...). I have verified this with print statements at the point where your patch is located.

The exit condition wont be executed if the thread is still stuck in the pthread_cond_timedwait

from libnetconf2.

Hi,
that makes sense. Could you please try it now, with the latest libnetconf2? Just a more polished version of your patch (I hope) so not a perfect solution but it should handle all systems with reasonable synchronization.

Regards,
Michal

from libnetconf2.

I like the fact that it only destroys things in the thread, however...

1. if nc_session_free is called after the closing test but before the end of the loop, things will never get freed.
2. a good exit (non failure) now returns 1 instead of 0
3. its probably not necessary to wait for thread closure at the end at all as the thread will close things automatically. As long it is disassociated from the registry, it doesn't matter. May be another race there but I suspect not.
4. not sure if CALLHOME flag locking is an issue as its not cleared on exit of loop at the end of the loop.
5. CALLHOME shoudl be cleared before the cond var and mutex are destroyed as the nc_session_free does not need to signal because it is already exiting before the free was called.

suggest 1 cleanup handler, returns 0 if CLOSING was true, clear CALLHOME FLAG first and optionally dont wait for thread closure at the end of nc_session_free because I think (not sure) its unnecessary you would have more deep knowledge on the guts in this respect. May be safer to wait as you are currently doing if not sure of the consequences.

from libnetconf2.

Hi,
I have rewritten it again, hopefully for the last time, what do you think about it now?

Regards,
Michal

from libnetconf2.

Sorry for the delay in responding. One last major issue as I see it.

• The mutex could still be locked when it is destroyed. This would be a bad thing. It needs a lock/unlock before the mutex/condvar are destroyed, just in case the thread is interrupted while the mutex is held.
• In nc_server_ch_client_thread_session_cond_wait, good return is now 1 and bad is -1. Previously it was good=1 and bad = 0 (or -1 if it didnt allocate a session). Is this intentional?

Destroying the mutex in free only is also good but you have to make sure its not being used any more in the thread.

from libnetconf2.

Hi,
fine, any other corner cases you would like covered?

Return value meaning changed slightly and it was intentional but I don't really see good/bad returns. -1 is returned on some unexpected libc function error, 1 is returned when the CH client configuration of the session was removed and 0 on any other return (basically session terminated for whatever reason, close-sesion, timeout, ...). I decided new session will not be established on -1 and 1 returns (I was thinking about -1, nothing would prevent retrying to establish a new session but then again, a function returned a fatal error so it would most likely always fail with an error).

Regards,
Michal

from libnetconf2.

Yes that will fix the issue.

Thanks for your patience. This is now a very robust solution for this functionality.

from libnetconf2.