Opened 12 years ago

Last modified 6 years ago

#1519 closed Patch

keepalive for FZ3rc1

Reported by: tommywu Owned by: Tim Kosse
Priority: normal Component: FileZilla Client
Keywords: Cc: tommywu, Tim Kosse
Component version: Operating system type:
Operating system version:

Description

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.)

Attachments (4)

filezilla_3_rc1_keepalive.patch (9.3 KB) - added by tommywu 12 years ago.
keepalive for FZ3rc1
filezilla3_rc1_keepalive2.patch (11.4 KB) - added by tommywu 12 years ago.
keepalive version 2
filezilla3_rc1_keepalive3.patch (9.8 KB) - added by tommywu 12 years ago.
keepalive version 3
filezilla_3.0.1_keepalive.patch (10.4 KB) - added by tommywu 12 years ago.
also keepalive for quickconnect

Download all attachments as: .zip

Change History (17)

Changed 12 years ago by tommywu

keepalive for FZ3rc1

comment:1 Changed 12 years ago by 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.

comment:2 Changed 12 years ago by Tim Kosse

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

comment:3 Changed 12 years ago by 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.

comment:4 Changed 12 years ago by 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.

Changed 12 years ago by tommywu

keepalive version 2

comment:5 Changed 12 years ago by 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

comment:6 Changed 12 years ago by 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.

comment:7 Changed 12 years ago by 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.

comment:8 Changed 12 years ago by 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)

Changed 12 years ago by tommywu

keepalive version 3

comment:9 Changed 12 years ago by 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

comment:10 Changed 12 years ago by 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;

comment:11 Changed 12 years ago by Tim Kosse

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

Changed 12 years ago by tommywu

also keepalive for quickconnect

comment:12 Changed 12 years ago by tommywu

File Added: filezilla_3.0.1_keepalive.patch

Note: See TracTickets for help on using tickets.