Expose libssh2_session_set_timeout()

[Jille]

issue #62

[Jille]

Not tested. I was hoping travis-ci would help me with that, but it doesn't seem to do much: https://app.travis-ci.com/github/php/pecl-networking-ssh2/pull_requests

[langemeijer]

Thank you for your time and your contribution. I have three points I'd like to discuss with you:

Point 1:
Triggered by your PR I started looking for the connect timeout (Which obviously you cannot set using your new ssh2_session_set_timeout() method because there is no session yet.) I found that as connect timeout value ssh2_connect() uses default-socket-timeout

I have always thought that this would be the default read/write timeout too, but apparently it is not. I think this is a bug. I think we have to add a libssh2_session_set_timeout() call to ssh2_connect(). Because the default value in PHP is 60 seconds, it's unlikely that this BC break would cause any problems for our users.

Point 2:
I'd like the function to be named ssh2_set_timeout(). The whole concept of libssh2 session and channels is not as such implemented in the ssh2 extension. For our users a session is just the name of the connection resource, most methods work on session.

Point 3:
I'd prefer that the function would follow the same parameters as stream_set_timeout:
ssh2_set_timeout(resource $session, int $seconds, int $microseconds = 0)

Although we have a problem here. set_stream_timeout() allows for microseconds, but libssh2_session_set_timeout() 'only' has a millisecond resolution. We would have to round $microseconds up to a multiple of 1000 and then divide by 1000.

I like the consistency between methods and also: Using sub-second precision is probably and edge-case. With my suggested call signature the seconds-case is made easy, still allowing for more precision when needed.

What do you think?

[langemeijer]

I have changed my mind on point 1. ssh2_exec($connection, "sleep 100;") would timeout at 60 seconds. Any PHP script running a task remotely for more than 60 without any output would change behaviour. I think that this is quite a normal use-case, think of dumping a database to file. I don't think having a default timeout is a good idea.

I tried messing about with libssh2_keepalive_config() but that didn't lead to anything useful.

[Jille]

Thanks for the elaborate response Casper. My apologies for the slow reply. Time is hard to find these days.

1. Agreed we should not make a change that'd give most people a 60s timeout. That'd break a lot of people unexpectedly.

2+3. Okay!

Groetjes,

[mrngm]

(apologies for the noise, it seems "Sync fork" on Github leaves a (somewhat unnecessary) merge commit)

[Jille]

Anything I can do to move this forward?

[ixlandia]

Why don't you call libssh2_session_set_timeout() with the socket timeout during php_ssh2_session_connect() and then call it again to set it back to 0 before returning? This would prevent infinite blocking during the ssh2_connect() call (which is what I'm encountering) but still allow the user to set their own timeout (or not) once they have a session. The only other way I can think of is to separate libssh2_session_init_ex() into a different PHP function from the rest of php_ssh2_session_connect() so that users have a way to set the timeout before it tries to connect and hangs there.

I ended up patching my own copy to just use the socket timeout for the entire session because I'm not doing anything that should take that long.