fix: FactoCord can get stuck waiting for Factorio to stop

[thecmdradama]

Simply triggering /quit again will resolve certain cases where FactoCord is waiting on Factorio to stop but Factorio has not been told to stop.

[maxsupermanhd]

Doesn't second /quit cause an emergency exit procedure? Or is that only for SIGTERMs?

[thecmdradama]

It does not seem to, At least from what I have been experiencing... I'm mainly running FactoCord through PufferPanel and without the change Factorio never receives the /quit command.

With the change when running FactoCord through PufferPanel:

4.943 Info ServerMultiplayerManager.cpp:726: Matching server connection resumed
2022.06.23 12:11:43: discord members update: 1616 members
DAEMON
 Server was told to stop
Waiting for factorio server to exit...
 605.693 Quitting: remote-quit.
...
 607.301 Info UDPSocket.cpp:218: Closing socket 
607.301 Info UDPSocket.cpp:248: Socket closed
607.386 Goodbye

Factorio server was closed, exit code 0
DAEMON
 Running post-execution steps

With the change when running FactoCord from CLI:

4.923 Info ServerMultiplayerManager.cpp:726: Matching server connection resumed
2022.06.23 12:21:57: discord members update: 1616 members
^C  12.420 Received SIGINT, shutting down
  12.420 Quitting: signal.
...
Waiting for factorio server to exit...
  13.915 Info UDPSocket.cpp:218: Closing socket
  13.915 Info UDPSocket.cpp:248: Socket closed
  13.986 Goodbye

[TechnoStrife]

I think a test on a production server is required. But I don't have one.

[maxsupermanhd]

So when you are stopping it via PufferPanel do you stop it via SIGINT or by sending a /quit to stdin? (Or are you doing rcon shiz?)

[thecmdradama]

@maxsupermanhd Nothing fancy... Just firing off SIGINT from PufferPanel.

[maxsupermanhd]

Just firing off SIGINT

I am aware that you should send /quit or do rcon. In regular environments (CTRL+C in tty) SIGINT is propagated to all process childs (foreground process group) so it should hit Factorio too. Probably some compatability issue.

Do note that FactoCord3 has functionality to still be running after Factorio exits and start it back again if hit with a start command. With your approach it will force FactoCord (and looks like only FactoCord) to quit too witch is not """intended""" behavior in my opinion.

Perhaps there should be a config option of some sort that will enable better compatibility with such panels. Maybe even a relaying SIGINT or other signals to Factorio if control panel is not capable to do so.

[maxsupermanhd]

Ideal option would be to reach some sort of interoperability with control panels maybe with custom API (tiny HTTP one for example). That will expose more abilities and eliminate such compatibility issues.

[thecmdradama]

Do note that FactoCord3 has functionality to still be running after Factorio exits and start it back again if hit with a start command.

This functionality is unaffected from my experiences as well. I'm familiar with the ability to stop, update, start, etc. Yeah there's probably a more elegant solution for it but it works and doesn't seem to have introduced any other issues.

[maxsupermanhd]

Now I am confused.

Let's start with beginning - what do you think should happen when you hit stop button on control panel? In my opinion it should stop internal Factorio server but leave FactoCord running to be able to turn it back on from Discord later.

As I already mentioned, ideal solution would have those actions split into 2 commands, one that rips whole service and one that sends internal shutdown command.

[thecmdradama]

Let's start with beginning - what do you think should happen when you hit stop button on control panel? In my opinion it should stop internal Factorio server but leave FactoCord running to be able to turn it back on from Discord later.

Ah, I think that's where we've gone down separate paths... I see and expect FactoCord to run as a wrapper service (for a lack of better term) for Factorio. So when I hit stop from PufferPanel, I'm expecting it to stop both Factorio and FactoCord. It shouldn't just stop Factorio. If I just want to stop Factorio, I'm expecting to manage that through Discord. But the option is there to pass in a shutdown or start command if required.

[maxsupermanhd]

IMHO PufferFish issue. SIGINT should be sent to foreground process group and not soul process that was launched. I am not sure about merging this because it can badly behave when Factorio shutdown was issued and before it had time to stop a SIGINT was hit on foreground process group making Factorio think that it needs to emergency exit and brick savefile for example.

I would merge this if such a behavior could be changed in config. (maybe something like "propagate_sigint_as_quit")

btw wrapper is a correct term

[thecmdradama]

Yeah no stress. PufferPanel doesn't really care or know about spawned child processes. Hence why this seemed like a straightforward solution to the problem.

[maxsupermanhd]

Closing due to inactivity, feel free to open another one with this being a configurable option and I will merge it.