keepalive for FZ3rc1

[tommywu]

This patch allow FileZilla 3 RC1 enable the keepalive function for FTP.
No option setting now, it will always enable keepalive for FTP, randomly send command every 30-60 seconds.

SFTP in FZ3 seems already with keepalive, so I just add this for FTP. (It's easy to enable it for another protocol, just modify the KeepAlive() for protocol's ControlSocket.)

[tommywu]

keepalive for FZ3rc1

[tommywu]

maybe we should also add "OPTS", "FEAT" to the keepalive command, and ignore the 5xx return code to make sure it will keep alive for pure-ftpd also.

[Tim Kosse]

FEAT is a bad idea due to the potentially large reply.

[tommywu]

forget OPTS and FEAT, and also other command or unknown command who cause 5xx return code.
pure-ftpd will ignore such command also, it still drop the connected after 30 minutes (default antiidle() time in pure-ftpd).

maybe just re-connect it after timeout, like filezilla 2.x.

[Tim Kosse]

Unfortunately I can't apply this patch in its current form.

• Keep-alive commands might take a while and engine will be flagged as busy for the whole time, blocking some user actions
• Keep-alive should be part of the engine itself
• The random TYPE A / TYPE I commands conflict with the TYPE cache. It should only use the currently active TYPE as keepalive command

It should be possible to implement this purely in CFtpControlSocket without the use of an active command id, see the m_repliesToSkip variable.

[tommywu]

keepalive version 2

[tommywu]

I move the timer from MainFrm to CFtpControlSocket, so it won't block UI now.
Also change the keepalive command, remove TYPE I/A, and add NOOP/MKD/RMD, and ignore the 5xx return code for them. Based on my test, it can keepalive for pure-ftpd now.
File Added: filezilla3_rc1_keepalive2.patch

[Tim Kosse]

This version has the same issue as the first one, it flags the engine at busy. And this time independent of user actions which will confuse all engine users.
Also, using MKD and RMD is a bad idea. For example some servers interpred RMD without argument to delete current directory.

[tommywu]

I'm confused with engine busy issue.
Engine means CFileZillaEngine class (m_pEngine)? or CControlSocket class (m_pControlSocket)?
2nd patch only using m_pEngine to send the enable/disable command for keepalive function. After send the enable command after build the connection, it will always using the timer for ControlSocket, when timer up, and no any packet send/receive for the socket in past 15 seconds, it will send the keepalive command.
If it's about m_pControlSocket busy, yes, it will cause it busy when we need to send keepalive command, but, for keepalive in ftp, that's what we're doing, using the controlsocket to sending noop command when it's idle to make the connection alive.

[Tim Kosse]

The problem is that you're using an actual command (in the sense of a CCommand object and it's associated command id), flagging the engine needlessly busy.

Imagine this: An engine user does nothing, thus expects the engine as idle. At some point it decides to issue a command. If by chance at the same time the engine is busy with the keepalive, the engine user will get FZ_REPLY_BUSY to his own command. This greatly confuses the engine user.
Likewise, the engine user will receive "command completed" notifications for commands he never issued himself (i.e. the keep alive commands), further adding to the confusion.

The proper solution is quite simple and can be implemented fully in ftpcontrolsocket.cpp without touching any other file and not involving CFileZillaEngine at all.

If the control socket is totally idle and it's time for the keepalive command, just Send() it and set m_repliesToSkip to one. We can do this since we're not interested in the reply to the keepalive command. And doing so has no impact on the externally observable behavior. (Despite logging notifications, but they can happen at any time anyhow)

[tommywu]

keepalive version 3

[tommywu]

But the CKeepAliveCommand (CCommand) object only used for once, I need tell ControlSocket to enable or disable keepalive from m_pState or MainFrm UI. After enable it, the system won't send CKeepAliveCommand again (only send it again if we need to disable it).

After enable it, all keepalive related procedure happened in ControlSocket itself.

I following the coding style for other command, so it work like (all in control socket):
OnTimer() -> DoKeepAlive() -> SendNextCommand() -> KeepAliveSend() -> Send() -> ParseResponse() -> KeepAliveResponse()

I merge them in the version 3 patch like:
OnTimer() -> DoKeepAlive() -> Send() -> ParseResponse() with m_repliesToSkip = 1

for CKeepAliveCommand, I still need it to send the enable/disable flag to ControlSocket (it just call once, and take a short time, just change the private/protected boolean variable). If we want total remove CKeepAliveCommand, we need to find another way to let connect command know to enable keepalive or not.

I also remove MKD/RMD from the keepalive command group, it look like STAT command is enough for pure-ftpd to keep the connection.
File Added: filezilla3_rc1_keepalive3.patch

[Tim Kosse]

Yes, CKeepAliveCommand needs to go.

Basically in your OnTimer method, just do this:

if (GetCurrentCommandId() == cmd_none && !m_pendingReplies && !m_repliesToSkip)
{

Send("NOOP"); Or whatever
m_repliesToSkip = 1;

}

And that's it! Nothing else to do, working keep-alive.
return;

[Tim Kosse]

Oh, and it's the connect command itself which should start the timer upon successful connection.

[tommywu]

also keepalive for quickconnect

[tommywu]

File Added: filezilla_3.0.1_keepalive.patch

[Tim Kosse]

I've finished implementing keepalive the proper way. Please see

​http://filezilla.svn.sourceforge.net/viewvc/filezilla/FileZilla3/trunk/src/engine/ftpcontrolsocket.h?r1=1806&r2=1805&pathrev=1806
​http://filezilla.svn.sourceforge.net/viewvc/filezilla/FileZilla3/trunk/src/engine/ftpcontrolsocket.cpp?r1=1806&r2=1805&pathrev=1806