Problem

Bug #3905 , when received a signal SIGTERM the directory cleanup is ignored due the execution not reach the 'clean up code'.

Solution

I wasn't able to move the 'clean up code' to make it available to both SIGTERM and SIGINT cases without side effects, so I created the function 'handle_signal' that handle both SIGTERM, SIGINT and other edge case for both Windows and Linux and realize the shutdown propertly.

To avoid race conditions a property was created to 'Server' class to know if was yet stopped.

Test

A shell script for a end-to-end test is available to validate the behaviour.
test-3905.txt

Considerations

• Perhaps the function 'handle_signal' should put outside the 'serve.py' file to avoid 'SOLID' principles violation
• The 'shutdown' function code perhaps should be put into the 'Server' class , method 'shutdown'.

It's ready, removed SIGQUIT and SIGUSR signals. Let me know if you need anything else.

• Sorry for take so long , It wasn't clear to me if was me that should remove the SIGQUIT and SIGUSR signals.
• Also, I accidentally merged no related code in the same pull request, so I had to made a 'force push'

As mentioned previously, could you add some tests @alvaro-osvaldo-tm?

Sure, until February 25th, tuesday, I will send the pull request.

Hi, the test was made

• I was careful for Windows cases , but still there is an issue I need to check in a windows machine, I don't have one at moment so I will do it this week.
• The test was in a new class 'Shutdown_by_signal_tests' because required an infrastructure to locate the 'mkdocs serve' directory. Because it's not possible to define the temporary directory as parameter in 'mkdocs server' shell command, so I need to check every '/tmp' directory to found.
• Exists many 'sleeps' and checks to prevent race conditions when running tests

I tried to make the test code readable , if need , I'm available for revision.

Thank you @alvaro-osvaldo-tm 🚀 Let me know when you've been able to handle the Windows issue you mentioned.

Hi, it take me a while.

For my surprise windows don't handle POSIX events smooth as Linux.

In windows it handle CTRL_C_EVENT using specific 'Windows API' instead to handle SIGTERM using 'Python's events' library.

I made this conclusion testing into an windows environment, reading some 'Python test cases' and more research.

I have a proposal to enable the cleanup feature for both POSIX and Windows , while keeping the implementation simple and maintainable.

It's use the factory pattern, the 'code' and the 'test' will request an implementation in the factories for the signal handling and receive one according the 'operation system'.

Behind the factory there will be two implementations, one for 'POSIX' and other for 'Windows' with the methods to
configure the 'shutdown callback' and other to dispatch a 'shutdown' signal for testing

Something like in the 'serve.py'

    def shutdown() -> None:
        if not server.is_active:
            return

        log.info("Shutting down...")

        try:
            server.shutdown()
        finally:
            config.plugins.on_shutdown()

        if isdir(site_dir):
            shutil.rmtree(site_dir)

event_handler = Event_Handler_Factory.new()
event_handler.registry_shutdown( lambda: shutdown )

And something like in 'shutdown_by_signal_tests.py' testing code:

event_handler = Event_Dispatch_Factory.new()
event_handler.shutdown( mkdocs_pid )

for signal in event_handler.get_supported_signals():
   event_handler.dispatch( mkdocs_pid , signal )
   ...
   self.assertTrue( was_cleaned )

And the factories make the selection for the correct implementation

class Event_Handler_Factory:

  @staticmethod
  def new() -> IEvent_Handler:

    if sys.platform == 'win32':
     return Event_Handler_For_Win32()

     return Event_Handler_For_POSIX()


class Event_Dispatch_Factory:

  @staticmethod
  def new() -> IEvent_Dispatch:

    if sys.platform == 'win32':
     return Event_Dispatch_For_Win32()

     return Event_Dispatch_For_POSIX()

Did you like I implement it ?
No problem if the decisions change after the implementation.

I ask because is slightly different from the current mkdocs code patterns.

Seems like a good plan to me 🙂 I'd like to get @tomchristie's opinion too.
Thank you so much for your work on this, and for your patience @alvaro-osvaldo-tm 🙏