With basic installation, the tray app always kills Jellyfin instead of safely stopping it

[qwerty12]

Version: 10.8.9
Operating System: Windows 11 21H2
Architecture: X64

Hi,

First of all, many thanks to you and the Jellyfin team for a wonderful media server.

I install Jellyfin in the recommended Basic Install mode. I noticed that whenever stopping Jellyfin via the tray app, C:\ProgramData\Jellyfin\Server\data would still contain jellyfin.db-shm and jellyfin.db-wal, indicating there was changes yet to be still written to jellyfin.db but couldn't occur because it was killed. Conversely, shutting down Jellyfin with the button on JF's dashboard appears to shut it down cleanly; the extra files mentioned previously aren't in that folder.

I see that the tray app tries to close the main window before killing the process, but Jellyfin has no window to send the message to (and even then, Jellyfin itself needs a message handler for WM_CLOSE), so it will always get killed. I changed the close mechanism to send Ctrl+C here instead and it works, but my code is crude and the reliability of the console APIs always seem to vary from Windows version to Windows version. (As an aside, another commit to the same repo adds a very-hacky-implemented listener for shutdown events so that Jellyfin has about ~10 seconds to safely exit before my PC finally gets powered off.)

It's my understanding that this won't happen in the service mode, because NSSM will also send Ctrl+C to Jellyfin. Thanks again.

[anthonylavado]

Oooh. Thank you for the example there. I've been meaning to add this in, and to be honest, as long as the commands work in Windows 10+, I think we're okay.

We're moving to .NET 7 with the next bigger release (10.9), so the server support will be for these OSes: https://github.com/dotnet/core/blob/main/release-notes/7.0/supported-os.md

Granted, this app is made with .NET Framework, but that's to avoid having to build & ship a bunch of data for what is essentially a tiny utility.

[qwerty12]

Oooh. Thank you for the example there. I've been meaning to add this in, and to be honest, as long as the commands work in Windows 10+, I think we're okay.

I think you should indeed be okay - not that I really know what I'm talking about here. I had problems some years ago with doing the same thing for another program but on reflection, I think that was a me problem, rather than one caused by Windows.

For what it's worth, I've been using the modified tray app that shuts down Jellyfin with GenerateConsoleCtrlEvent for a month and a bit now and I've not run into any problems. The only problem I can think of is that sending Ctrl+C isn't technically guaranteed to shut down Jellyfin, so I ended up adding bad code in the tray app to kill Jellyfin after ten seconds but I don't think I've noticed it even being hit in practice.

We're moving to .NET 7 with the next bigger release (10.9), so the server support will be for these OSes: https://github.com/dotnet/core/blob/main/release-notes/7.0/supported-os.md

Save for communicating with Jellyfin via the REST API, GenerateConsoleCtrlEvent might indeed be the best way to cleanly shutdown Jellyfin IMO, as Jellyfin itself already handles Ctrl+C cleanly so no changes are required there, plus GenerateConsoleCtrlEvent etc. is provided by Windows itself, going as far back as NT 4 I think, and I can't imagine .NET's DllImport functionality ever going away.

[Ede123]

As a thought: Does it even make sense to stop Jellyfin when closing the tray app?

For me I'd see the tray app only as a graphical frontent for the server running in the background. When I close the graphical frontend, there's no need to stop the server at all.

As a matter of fact I just came here when I accidentally closed the tray app and was wondering why Jellifin had shut down as well.

[anthonylavado]

@Ede123 Yes, it does make sense. There is no built in "shutdown" button for the web interface on Windows (unlike Linux) because it requires being able to run a script and a few other wrinkles we couldn't really sort out in Windows.

We can investigate it, but I don't feel sure that it will make it into this next release for the end of April (there's quite a lot to cover across all projects).

If you wanted a background service not tied to the tray app today, you'd need Jellyfin to be running in the service mode. It's a choice made during the installation, and there is no good way to convert from one to the other. It's labelled as "Advanced" because involves a lot of unique permission challenges for accessing network modes.

Could I suggest hiding the tray app from the notification area in the meanwhile? 🙂

[Ede123]

Thanks, that does make sense.

Personally I'm fine with the current solution, just wanted to note that it might be counter-intuitive when compared to some other server applications (also on windows), and that it might cause "accidents" Like in my case, when I wanted to close another app from the tray and accidentally killed the Jellyfin server instead.

It's a good pointer that the service would behave differently - maybe I'll actually try that at one point. (Didn't dare to do so, since it's specifically recommended against in multiple places, but it might actually align more with what I'd personally be expecting personally in a server app.

As a thought: Maybe The tray app could control Jellyfin via some sort of RPC? Then it could maybe be decoupled from the server itself and shutting down the server via other means might be easier as well?

In any case, certainly none of these is particularly urgent (functionality is all there as it is)! Thanks for the great software!

[anthonylavado]

@qwerty12 Looking at your fork, you seem to have a lot of nice changes, like holding at shutdown/etc. Would you be open to making a pull request? Keep in mind version 10.9 is now being released so the files are... different. Some updates would be needed.

[qwerty12]

@anthonylavado

Oh, congratulations on the release :-)
I think I'll end up rebasing anyway, I messed up the merging.

Fair warning: the code quality of my changes isn't exactly top notch

Leaving out changes that make sense only for me, and those that rely on heavily on .NET internals remaining the same for not a lot of benefit, I think these might be worth looking at:

Small changes:

• Use ShellExecute to open browser and log folder - functions exactly the same, but a little bit faster

• Only call CheckShowServiceNotElevatedWarning if running as service

• Don't set WindowStyle

Larger changes:

• Check if existing JF process is our JF + Keep handle to JF process + track state with that - avoiding enumerating existing processes should be faster

• Kindly request JF's termination; don't kill it - the main one for me. For what it's worth, for over a year, I've not had any issues stopping Jellyfin by sending Ctrl+C to the process, though I think it only works when the same Jellyfin.Windows.Tray.exe process is the one to start jellyfin.exe

  • Hack: kill Jellyfin after 10 seconds if not stopped - if sending Ctrl+C fails, kill Jellyfin anyway (if not exiting the tray program). I don't really like how this is implemented, but I don't know any better

  • Wait up to 10 secs to stop JF on Windows shutdown - what it says on the tin. This isn't worth looking at without having the tray application send Ctrl+C, as the only thing this would then change would be the tray application killing jellyfin.exe at shutdown instead of Windows. Currently, it uses Reflection to get the tray icon's HWND; the reliance on something internal to .NET can be replaced by creating a non-visible NativeWindow

Opinionated changes:

• Explicitly set WorkingDirectory to JF's install dir - I think it's better to be explicit, but someone somewhere may rely on the started jellyfin.exe inheriting the Jellyfin.Windows.Tray.exe process's working directory

• Restart Jellyfin automatically if it crashes (as long as it's been running for at least a minute) - this is useful to me, but this makes the tray application more like a service manager, and there might be a non-crash-related reason for Jellyfin exiting with a non-zero status code

Let me know what to discard, and I'll try and make PRs for the rest.

EDIT: Changes rebased on master: https://github.com/qwerty12/jellyfin-server-windows/tree/10.9-rebase