Bug#828018: minetest-server is not shutdown properly

2016-06-24 Thread Markus Koschany
Control: tags -1 - moreinfo

On 24.06.2016 08:32, dr...@dr4ke.net wrote:
[...]
>> Restarting the server is equivalent with stopping and starting the
>> server again. Systemd will send a SIGTERM signal by default to the
>> server to terminate the running process. I don't see a reason why we
>> should use a different KillSignal in this case. SIGINT means
>> interrupting the process.
> 
> 
> You're right, there is probably a bad choice of signal processing
> in the minetest-server code. I'll report an issue there too.
> 

I have reported this issue upstream at

https://github.com/minetest/minetest/issues/4251

You might want to follow up there. I think we can apply your proposed
patch for now, although it would be preferable that the server responded
to SIGTERM signals correctly.

Regards,

Markus



signature.asc
Description: OpenPGP digital signature


Bug#828018: minetest-server is not shutdown properly

2016-06-24 Thread Martin Quinson
The proposed patch sounds very reasonable to me. I'd apply it.

Bye, Mt.

- Mail original -
> On 2016-06-23 23:09, Markus Koschany wrote:
> > What minetest modification caused the server failure? How can we
> > reproduce the data inconsistency?
> 
> 
> There is no server failure. The failure is happening when systemd sends
> a
> SIGTERM. It seems that minetest-server respond by exiting immediately
> without executing its shutdown routines, namely the register_on_shutdown
> functions from the mods.
> I experienced the issue with one particular mod that only save its data
> through this shutdown function (this is an error, IMO, and I reported it
> to the mod developper).
> The mod is: https://github.com/minetest-mods/character_creator
> https://forum.minetest.net/viewtopic.php?f=9=13138
> 
> To reproduce:
> - install the mod
> - change your appearance in the game
> - leave the game
> - restart the minetest-server service with systemctl
> - enter the game, you have the default appearance
> 
> At shutdown, the file character_creator.mt should be created in the
> world directory. It is created if you kill the minetest-server from
> the game with /shutdown or outside with a SIGINT (I think it's the
> same as a ctrl-C, right?)
> 
> 
> > Restarting the server is equivalent with stopping and starting the
> > server again. Systemd will send a SIGTERM signal by default to the
> > server to terminate the running process. I don't see a reason why we
> > should use a different KillSignal in this case. SIGINT means
> > interrupting the process.
> 
> 
> You're right, there is probably a bad choice of signal processing
> in the minetest-server code. I'll report an issue there too.
> 
> 
> > I suggest to investigate the cause of the server failure and then
> > report
> > your findings to the mod developers. If they think it is rather a
> > minetest bug, please get in contact with the minetest developers
> > directly because this is the fastest way to get your issue solved.
> 
> 
> It's already done with the mod. I'll do it with the server, regarding
> its
> behavior with the SIGTERM signal.
> 
> 
> > If you want to play with different service file settings, use a
> > modified
> > service file in /etc/systemd/system to override Debian's default
> > settings.
> 
> 
> That's what I do for now.
> 
> Thank you for these suggestions.
> 
> 



Bug#828018: minetest-server is not shutdown properly

2016-06-24 Thread dr4ke

On 2016-06-23 23:09, Markus Koschany wrote:

What minetest modification caused the server failure? How can we
reproduce the data inconsistency?



There is no server failure. The failure is happening when systemd sends 
a

SIGTERM. It seems that minetest-server respond by exiting immediately
without executing its shutdown routines, namely the register_on_shutdown
functions from the mods.
I experienced the issue with one particular mod that only save its data
through this shutdown function (this is an error, IMO, and I reported it
to the mod developper).
The mod is: https://github.com/minetest-mods/character_creator
https://forum.minetest.net/viewtopic.php?f=9=13138

To reproduce:
- install the mod
- change your appearance in the game
- leave the game
- restart the minetest-server service with systemctl
- enter the game, you have the default appearance

At shutdown, the file character_creator.mt should be created in the
world directory. It is created if you kill the minetest-server from
the game with /shutdown or outside with a SIGINT (I think it's the
same as a ctrl-C, right?)



Restarting the server is equivalent with stopping and starting the
server again. Systemd will send a SIGTERM signal by default to the
server to terminate the running process. I don't see a reason why we
should use a different KillSignal in this case. SIGINT means
interrupting the process.



You're right, there is probably a bad choice of signal processing
in the minetest-server code. I'll report an issue there too.


I suggest to investigate the cause of the server failure and then 
report

your findings to the mod developers. If they think it is rather a
minetest bug, please get in contact with the minetest developers
directly because this is the fastest way to get your issue solved.



It's already done with the mod. I'll do it with the server, regarding 
its

behavior with the SIGTERM signal.


If you want to play with different service file settings, use a 
modified
service file in /etc/systemd/system to override Debian's default 
settings.



That's what I do for now.

Thank you for these suggestions.



Bug#828018: minetest-server is not shutdown properly

2016-06-23 Thread Markus Koschany
Control: forwarded -1 https://github.com/minetest/minetest/issues/4251

On 24.06.2016 03:01, Matthew Bekkema wrote:
> On Thu, Jun 23, 2016 at 11:09:59PM +0200, Markus Koschany wrote:
>> Restarting the server is equivalent with stopping and starting the
>> server again. Systemd will send a SIGTERM signal by default to the
>> server to terminate the running process. I don't see a reason why we
>> should use a different KillSignal in this case. SIGINT means
>> interrupting the process.
> 
> I took a quick look through the code, it looks like they only add a handler 
> for
> SIGINT:
> 
> https://github.com/minetest/minetest/blob/095f623fa7278375a6fa4fe01ed39d2b68cce7af/src/porting.cpp#L97

This seems to be true. I have forwarded this issue upstream and I think
minetestserver should support SIGTERM signals as well.

I have tested the KillSignal=SIGINT option which appears to work but the
current behavior works for me as well.




signature.asc
Description: OpenPGP digital signature


Bug#828018: minetest-server is not shutdown properly

2016-06-23 Thread Matthew Bekkema
On Thu, Jun 23, 2016 at 11:09:59PM +0200, Markus Koschany wrote:
> Restarting the server is equivalent with stopping and starting the
> server again. Systemd will send a SIGTERM signal by default to the
> server to terminate the running process. I don't see a reason why we
> should use a different KillSignal in this case. SIGINT means
> interrupting the process.

I took a quick look through the code, it looks like they only add a handler for
SIGINT:

https://github.com/minetest/minetest/blob/095f623fa7278375a6fa4fe01ed39d2b68cce7af/src/porting.cpp#L97



Bug#828018: minetest-server is not shutdown properly

2016-06-23 Thread Markus Koschany
Control: tags -1 moreinfo

On 23.06.2016 21:42, dr4Ke wrote:
> Package: minetest-server
> Version: 0.4.14+repack-3
> Severity: normal
> Tags: patch
> 
> Dear Maintainer,
> 
> *** Reporter, please consider answering these questions, where appropriate ***
> 
>* What led up to the situation?
> Restarting the minetest-server service
> 
>* What exactly did you do (or not do) that was effective (or
>  ineffective)?
> I just restarted the minetest-server service
> 
>* What was the outcome of this action?
> Some mod data was not present at the next boot, because of two combined 
> issues:
> - the mod is saving its data with register_on_shutdown which is not safe from
>   a server failure
> - the systemd service kills the minetest-server process instead of shuting it
>   down properly.
> 
>* What outcome did you expect instead?
> We can shutdown the minetest-server properly with a kill signal: SIGINT.
> 

What minetest modification caused the server failure? How can we
reproduce the data inconsistency?

Restarting the server is equivalent with stopping and starting the
server again. Systemd will send a SIGTERM signal by default to the
server to terminate the running process. I don't see a reason why we
should use a different KillSignal in this case. SIGINT means
interrupting the process.

I suggest to investigate the cause of the server failure and then report
your findings to the mod developers. If they think it is rather a
minetest bug, please get in contact with the minetest developers
directly because this is the fastest way to get your issue solved.

If you want to play with different service file settings, use a modified
service file in /etc/systemd/system to override Debian's default settings.

Regards,

Markus




signature.asc
Description: OpenPGP digital signature


Bug#828018: minetest-server is not shutdown properly

2016-06-23 Thread dr4Ke
Package: minetest-server
Version: 0.4.14+repack-3
Severity: normal
Tags: patch

Dear Maintainer,

*** Reporter, please consider answering these questions, where appropriate ***

   * What led up to the situation?
Restarting the minetest-server service

   * What exactly did you do (or not do) that was effective (or
 ineffective)?
I just restarted the minetest-server service

   * What was the outcome of this action?
Some mod data was not present at the next boot, because of two combined issues:
- the mod is saving its data with register_on_shutdown which is not safe from
  a server failure
- the systemd service kills the minetest-server process instead of shuting it
  down properly.

   * What outcome did you expect instead?
We can shutdown the minetest-server properly with a kill signal: SIGINT.


PATCH:
--- /lib/systemd/system/minetest-server.service 2016-05-18 09:48:48.0 
+0200
+++ minetest-server.service 2016-06-23 21:40:17.804380123 +0200
@@ -9,6 +9,7 @@
 User=Debian-minetest
 Group=games
 ExecStart=/usr/lib/minetest/minetestserver --config 
/etc/minetest/minetest.conf --logfile /var/log/minetest/minetest.log
+KillSignal=SIGINT
 
 [Install]
 WantedBy=multi-user.target
--- /lib/systemd/system/minetest-server@.service2016-05-18 
09:48:48.0 +0200
+++ minetest-server@.service2016-06-23 21:40:24.632157955 +0200
@@ -9,6 +9,7 @@
 User=Debian-minetest
 Group=games
 ExecStart=/usr/lib/minetest/minetestserver --config /etc/minetest/%i.conf 
--logfile /var/log/minetest/%i.log
+KillSignal=SIGINT
 
 [Install]
 WantedBy=multi-user.target
END_OF_PATCH



-- System Information:
Debian Release: stretch/sid
  APT prefers testing
  APT policy: (500, 'testing')
Architecture: amd64 (x86_64)

Kernel: Linux 4.5.0-1-amd64 (SMP w/4 CPU cores)
Locale: LANG=fr_FR.UTF-8, LC_CTYPE=fr_FR.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash
Init: systemd (via /run/systemd/system)

Versions of packages minetest-server depends on:
ii  adduser  3.114
ii  init-system-helpers  1.29
ii  libc62.22-7
ii  libcurl3-gnutls  7.47.0-1
ii  libgcc1  1:5.3.1-14
ii  libjsoncpp1  1.7.2-1
ii  libleveldb1v51.18-5
ii  libluajit-5.1-2  2.0.4+dfsg-1
ii  libsqlite3-0 3.12.1-1
ii  libstdc++6   5.3.1-14
ii  minetest-data0.4.14+repack-3
ii  zlib1g   1:1.2.8.dfsg-2+b1

minetest-server recommends no packages.

minetest-server suggests no packages.

-- Configuration Files:
/etc/minetest/minetest.conf changed [not included]

-- no debconf information