MinecraftServerControl / mscs

Powerful command-line control for UNIX and Linux powered Minecraft servers
https://minecraftservercontrol.github.io
BSD 2-Clause "Simplified" License
485 stars 62 forks source link

Wait until query port opens before attempting to query #242

Closed Roflicide closed 4 years ago

Roflicide commented 4 years ago

Hi,

Attempting to query a world before the query port is open results in the socat "connection" (not really a connection since it's UDP) being dropped. Additionally, the code in lines 1734-1736 is never triggered -- even if the query connection failed, the error message will never print.

  # Verify the connection to the query server worked.   
  if [ $? -ne 0 ]; then 
    printf "Error connecting to the query server.\n"    
  fi

Subsequent queries to the world will result in a running (query server offline) message because the socat connection will never be re-established.

Isssue #232 fixes this issue downstream by having an else clause that executes the queryStart() function again. However, this is only triggered when the socat connection has dropped at least once, so it's a bit inefficient.

This pull request fixes the root issue by waiting until the minecraft query port is open through the use of netcat (I assume most linux distros ship with netcat so this shouldn't be a dependency issue, but if there's a way to do the same thing with socat than that may be better). Once this query port is detected as open, it creates the query.in and query.out files and establishes the socat connection. By doing it this way, we can detect if a) the world is still starting and that's why we're not getting queries or b) there is something actually wrong with the query server even though the port is open (which is what is implied in Issue #169, though in my testing, this has never happened).

I'm leaving the code in from #232 (thanks to @sheimer for the PR ! ) as a redundancy in case the query server drops even after the query ports are open (again, as is implied in #169).

sandain commented 4 years ago

@Roflicide thanks for the patch, but I'm going to give this a NAK. There is a BSD netcat and a GNU netcat that do not have compatible command line arguments. If we are going to switch back to using netcat, we need to incorporate code that detects which version is being run so that the call can be made correctly for both versions.

sandain commented 4 years ago

I just read the socat docs, and it looks like your call to netcat could be replaced with something like:

- while ! nc -u -z $SERVER_IP $QUERY_PORT; do
+ while ! echo | $SOCAT - UDP-CONNECT:$SERVER_IP:$QUERY_PORT >/dev/null 2>&1; do

World on 25667 not running and world on 25668 running:

$ echo | socat - UDP-CONNECT:127.0.0.1:25567 >/dev/null 2>&1; echo $?
1
$ echo | socat - UDP-CONNECT:127.0.0.1:25568 >/dev/null 2>&1; echo $?
0
Roflicide commented 4 years ago

@Roflicide thanks for the patch, but I'm going to give this a NAK. There is a BSD netcat and a GNU netcat that do not have compatible command line arguments. If we are going to switch back to using netcat, we need to incorporate code that detects which version is being run so that the call can be made correctly for both versions.

I didn't know this was thing, good catch

I just read the socat docs, and it looks like your call to netcat could be replaced with something like:

- while ! nc -u -z $SERVER_IP $QUERY_PORT; do
+ while ! echo | $SOCAT - UDP-CONNECT:$SERVER_IP:$QUERY_PORT >/dev/null 2>&1; do

World on 25667 not running and world on 25668 running:

$ echo | socat - UDP-CONNECT:127.0.0.1:25567 >/dev/null 2>&1; echo $?
1
$ echo | socat - UDP-CONNECT:127.0.0.1:25568 >/dev/null 2>&1; echo $?
0

nice solution, i tested this and it works! ill add another commit

sandain commented 4 years ago

@Roflicide thanks for the patch, but I'm going to give this a NAK. There is a BSD netcat and a GNU netcat that do not have compatible command line arguments. If we are going to switch back to using netcat, we need to incorporate code that detects which version is being run so that the call can be made correctly for both versions.

I didn't know this was thing, good catch

I didn't either until it bit me! MSCS used to use netcat in the first iteration of the query code

sandain commented 4 years ago

See Issue #25 RE the switch to socat

sandain commented 4 years ago

I just tried this out and it works nicely. I'll go ahead and accept the PR. Thanks for the work on this @Roflicide