Pryaxis / TShock

☕️⚡️TShock provides Terraria servers with server-side characters, anti-cheat, and community management tools.
GNU General Public License v3.0
2.41k stars 377 forks source link

Add tshock installer executable #2840

Closed SignatureBeef closed 1 year ago

SignatureBeef commented 1 year ago

This aims to fix a lot of the "the console just closes" issues we've been seeing recently by adding a second executable, ./TShock.Installer[.exe], to the archive for those who do not have dotnet installed globally on their system.

The installer will download the dotnet 6.0.11 runtime to a local ./dotnet folder, it will then set the DOTNET_ROOT environment variable that ./TShock.Server will proceed to use to run the non-self contained version.

Why not just make TShock.Server be self contained?

  1. Because many people have expressed they do not want dotnet in every installation, especially when considering GSP's whom already have it installed globally.
  2. MonoMod has had issues with jit hooks, at least on arm64 when in self-contained + single file modes. however, this can be explored further due to recent changes, or via the new reorg branch which may allow clrjit to be found easier (at least for custom setups) which was difficult in the current version. alternatively, single file mode should just be avoided at the expense of more ./bin redirections, but point 1 takes precedence so this is not really effective either way.

Will TShock.Server be affected? No, it remains the same, the Installer will just setup dotnet, and call the existing setup which remains a singlefile but not self-contained (TShock.Server[.exe]).

Can TShock.Server not be a single file so TShock.Server.dll and others are extracted? Yes, but out of scope for this PR (and mainly just a flag in CI if someone wanted to change that later)

Why do you hardcode the urls? Because we don't support .net7 yet, nor did i want to overcomplicate the downloading using APIs. Happy for someone to do this, and ive left the reference to dotnet-install.[ps1/sh] which can be used to do this for when security/patches are released, but for now, 6.0.11 is known to work.

Can TShock.Installer[.exe] be called x instead? Yes, i have no emotional attachment, i only cared about the actual functionality, so happy to rename, or someone can rename after merging if there is some preference later before a release/doco.

note, i would suggest updating doco if this gets merged to make people aware of this, but if they read the docs they would know to install dotnet to begin with, so hopefully by naming it TShock.Installer, they will try it by nature, and thus will download dotnet for them...

Below are what the installer looks like on various OS's:

note, ignore TShock Installer 6.0.0.0 being printed, fixed in a recent commit to show 5.0.0

Linux ARM64 / RPI image

OSX x64 Intel: image

Ubuntu x64 (ubuntu-22.10-live-server-amd64): VirtualBox_Ubuntu_21_11_2022_09_34_39

Windows 11 x64: Screenshot 2022-11-21 094503

SignatureBeef commented 1 year ago

This is really cool and a good solution to Microsoft's unfortunate approach (or lack there of) to "informing the end user".

Is it intended that TShock.Installer.pdb is included with the CI artifacts?

Unfortunately, this sorta breaks the SIGINT handler the server has... or at least, makes them work different than intended & advertised, and causes exceptions.

At world selection, sending SIGINT will kill the installer process, but not the server process. Pressing Ctrl + C afterwards will not do anything (which is expected to force shutdown), and the server will stay in the background. Inserting anything into the terminal will cause the following exception:

System.IO.IOException: Input/output error
   at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirectory, Func`2 errorRewriter)
   at Interop.CheckIo(Int64 result, String path, Boolean isDirectory, Func`2 errorRewriter)
   at System.IO.StdInReader.ReadKey()
   at System.IO.StdInReader.ReadLineCore(Boolean consumeKeys)
   at System.IO.StdInReader.ReadLine()
   at System.IO.SyncTextReader.ReadLine()
   at System.Console.ReadLine()
   at Terraria.Main.DedServ()
   at Terraria.Program.orig_RunGame()

If instead you select a world and let it load, sending SIGINT will begin shutting down properly, and eventually exit. However, like above, you will be unable to force shutdown (because you cannot send a second SIGINT), and inserting anything into the terminal whilst the server process is still alive will cause the same exception as above.

The PDB should probably be embedded like the server does, 403677fad2c11d0a29230e19a3edf04e7d325b64 fixes that.

SIGINT / CTRL + C, may now be sorted as per: fbd82bbce8802b0b5a10f836410aeaa7de0c0ef1 On windows at least, it seems to allow TShock to handle the Console.CancelKeyPress event which should allow the sub process to exit as expected.

SignatureBeef commented 1 year ago

Yup, PDB is now embedded and the SIGINT handler works as expected.

In my testing of screwing with the SIGINT handler, I interrupted (and corrupted) the download of the .NET runtime archive, which made successive executions always throw with ICSharpCode.SharpZipLib.SharpZipBaseException: Unexpected EOF, until I deleted the .NET runtime archive.

Would it make sense to catch any exceptions thrown when unarchiving, and delete the downloaded file?

Yeh of course, i quickly done e561158699286042154e4b0f8feeac2dabba4853 which should now account for this.

Using a corrupt runtime archive: image

After the catch cleans it up, allows a new download: image