cosullivan / SmtpServer

A SMTP Server component written in C#
MIT License
676 stars 160 forks source link

Graceful restart of server #121

Closed large closed 3 years ago

large commented 4 years ago

Hi @cosullivan thank you for the this awesome library :) This is more a question than a issue.

How do you ensure a graceful restart of the server? (Server needs to be restarted when user change settings related to the SMTP server)

I have the following code

//(...)
//Startup
            serverTask = server.StartAsync(cancellationTokenSource.Token);

//(...)
//Kill
            cancellationTokenSource.Cancel();
            try
            {
                serverTask.Wait();
            }
            catch { }

Are all ongoing SMTP clients gracefully ended on this method? If an client just sits there, does it wait until NetworkBufferReadTimeout or CommandWaitTimeout are triggered?

cosullivan commented 4 years ago

Hi @large,

Cancelling the CancellationToken should gracefully close the connections but it wont gracefully close the sessions.

For example, a client could be connected and be in the middle of a session and if the cancellation token is cancelled then they will be disconnected mid session. It wont wait for them to complete the session (by sending the QUIT command) but in theory it should be fine to disconnect them mid session because the mail clients will be notified of the failure and the user will resent or all good Mail Transfer Agents should be buffering the message until a successful completion too.

What are the settings that you want to change?

Thanks, Cain.

large commented 4 years ago

Thank you for the feedback @cosullivan . Your description is the behaviour I see in my tests. I agree that closing the ports should work just fine, but I want a more gentle way.

Here is how I see the scenario:

  1. Close new accepts (done with a IEndpointListener, I belive) to ensure no more sessions.
  2. Wait for sessions to complete and probably a max time variable (for the idiotic NOP clients)
  3. Then trigger the cancellationTokenSource.Cancel();
  4. Restart server with new parameters

This is probably solvable as SmtpServer class is today, but it could be a feature request?

cosullivan commented 4 years ago

Hi @large,

Yes, it sounds like it probably needs a Shutdown method on the server.

I have some ideas on how to handle this so will take a look over the next week.

Thanks, Cain.

large commented 4 years ago

Hi @cosullivan , was this included in the current release? My current method kind of works, but usually your methods are better ;)

cosullivan commented 4 years ago

Hi @large ,

Sorry that wasn't included.

I have a separate branch where I started playing around for that and I should have some time this weekend to get back to it.

Thanks, Cain.

large commented 4 years ago

Thank you for the feedback @cosullivan 👍 I have a running alpha of my software for testing and your library is stable. Looking forward for this method fixed. Keep up the good work :)

cosullivan commented 4 years ago

Hi @large ,

I've pushed my branch here; https://github.com/cosullivan/SmtpServer/tree/feature/Shutdown

I've added a Shutdown() method on the server, so the idea is that you can call the Shudown method and this will stop the listener but any active sessions will continue until they are completed. Waiting on the Task returned by the StartAsync() method will return once all of those sessions are complete as it currently does.

If you want to forcibly shutdown then you can cancel the CancellationToken passed into the StartAsync method as this will shutdown the listener and also cancel the active sessions.

In theory, after you call the Shutdown method you should be able create a new instance of the server and start it back up using the same ports so the active sessions will complete using the old settings and the server will start accepting new sessions with the updated settings.

I think I may need to make the Shutdown method return a Task that you can wait on such that you know when the listeners have been stopped in case you try to start a new server too quickly.

Let me know what you think so far?

EDIT: I've now added a ShutdownTask that will complete when the endpoint listeners are no longer accepting new sessions.

cosullivan commented 4 years ago

Hi @large ,

I've now published a new version 7.2.0 that contains this Shutdown method.

Additionally, there are two new examples showing the difference between cancelling the server via the CancellationToken and performing a graceful shutdown.

https://github.com/cosullivan/SmtpServer/blob/master/Src/SampleApp/Examples/ServerCancellingExample.cs

https://github.com/cosullivan/SmtpServer/blob/master/Src/SampleApp/Examples/ServerShutdownExample.cs

Thanks, Cain.

large commented 3 years ago

The gracefull shutdown works as expected, thank you for the examples 👍🏼

cosullivan commented 3 years ago

Thank you for the feedback @cosullivan 👍 I have a running alpha of my software for testing and your library is stable. Looking forward for this method fixed. Keep up the good work :)

If there are any details you can share about your project then I am always keen to hear how people are using it. If its something more private then you can email me directly.

Thanks, Cain.