Fexty12573 / SharpPluginLoader

A C# plugin loader for Monster Hunter World
MIT License
32 stars 2 forks source link

[Help Wanted/Bug?] Handling of Extra Threads on Plugin Reload #29

Closed GreenComfyTea closed 5 months ago

GreenComfyTea commented 6 months ago

Describe The Bug

I don't know if SPL is supposed to do anything with additional threads, but the way it works right now is that if a plugin spawns a new thread, the thread will persist after reloading. New plugin instance will spawn it's own thread too, so as a result there will be 2 threads existing at the same time.

This happens with Task.Run():

Task.Run(() => {
    while (true)
    {
        Log.Info($"Task.Run() Thread: {Thread.CurrentThread.ManagedThreadId}");
    }
});

After 2 reloads:

image

New threads can spawn implicitly. FileSystemWatcher fires it's events in a separate thread, and it persists after a reload too.

The result is this mess:

Screenshot ![image](https://github.com/Fexty12573/SharpPluginLoader/assets/30152047/3bba2901-b89c-435f-ae82-951e761ccf30)

PS. Also it seems that SPL fires events for reloaded plugins before OnLoad(). This does not happen without reloading, when the game is launched from zero.

public void OnLoad()
{
    localizationManager.Init();
    configManager.Init();
}

public void OnImGuiFreeRender()
    {
        // Access data in localizationManager or configManager
        // ---> Exceptions
    }

To Reproduce

1) Make a plugin with FileSystemWatcher or with Thread.Run() containing repeating console output; 2) Launch the game; 3) Hot-reload the plugin; 3) Observe.

Expected Behavior

If it is a bug with SPL, it should be fixed. Otherwise, maybe there should be IPlugin.OnUnload() event where plugins can dispose of resources and threads, and finalize gracefully.

Priority

Suggested P3 Priority. The issue affects the development of plugins.

PC Specs

Show/Hide - `Motherboard`**: Asus ROG Strix Z390-H Gaming** - `CPU`**: Intel Core i9-9900k** - `RAM`**: 32 GB 3200MHz** - `GPU`**: Nvidia GeForce RTX 3090 Ti** - `System and Game SSD`**: Samsung SSD 970 EVO Plus NVMe M.2 1TB** - `Plugin Repo SSD`**: Western Digital Blue SATA M.2 2280 2TB**

Environment

Show/Hide - `OS`: **Window 10 Enterprise Version 22H2 (OS Build 19045.3996)** - `Monster Hunter: World`: **v15.21.00** - `SharpPluginLoader`: **pre-v0.0.4.0 (a66b29affe5f48e06f21247dd7247b208c72acf5)** - `Nvidia Drivers`**: v551.52**

Game Display Settings

Show/Hide - `DirectX 12 API`**: On** - `Screen Mode`**: Borderless Window** - `Resolution Settings`**: 2880x1620** (Supersampled down to 1920x1080) - `Aspect Ratio`**: Wide (16:9)** - `Nvidia DLSS`**: Off** - `FidelityFX CAS + Upscaling`**: Off** - `Frame Rate`**: No Limit** - `V-Sync`**: On**

Game Advanced Graphics Settings

Show/Hide - `Image Quality`**: High** - `Texture Quality`**: High Resolution Texture Pack** - `Ambient Occlusion`**: High** - `Volume Rendering Quality`**: Highest** - `Shadow Quality`**: High** - `Capsule AO`**: On** - `Contact Shadows`**: On** - `Anti-Aliasing`**: Off** - `LOD Bias`**: High** - `Max LOD Level`**: No Limit** - `Foliage Sway`**: On** - `Subsurface Scattering`**: On** - `Screen Space Reflection`**: On** - `Anisotropic Filtering`**: Highest** - `Water Reflection`**: On** - `Snow Quality`**: High** - `SH Diffuse Quality`**: High** - `Dynamic Range`**: 64-bit** - `Motion Blur`**: On** - `DOF (Depth of Field)`**: On** - `Vignette Effects`**: Normal** - `Z-Prepass`**: On**

Mods and External Tools:

Show/Hide - [RTSS Overlay v7.3.4](https://www.guru3d.com/page/rivatuner-rtss-homepage/) - [Reshade 6.0.0](https://reshade.me/) with SMAA.fx - [Stracker's Loader v3.0.1](https://www.nexusmods.com/monsterhunterworld/mods/1982) - [Performance Booster and Plugin Extender v1.3](https://www.nexusmods.com/monsterhunterworld/mods/3473) - [HunterPie v2.10.0.134](https://www.nexusmods.com/monsterhunterrise/mods/181) - [HelloWorld (an overlay that shows lots and lots of stuff) v10.4](https://www.nexusmods.com/monsterhunterworld/mods/142) - [Bow Spread Pattern Fix v1.0](https://www.nexusmods.com/monsterhunterworld/mods/6914) - [Tic Rate Fix v1.0](https://www.nexusmods.com/monsterhunterworld/mods/3474) - [Guiding Lands Gathering Indicator v1.12.11.01](https://www.nexusmods.com/monsterhunterworld/mods/1986) - [Gems Sorted by Name (Alphabetized Gems) v1.6.1](https://www.nexusmods.com/monsterhunterworld/mods/2364) - [Reshade Hook v1.0](https://www.nexusmods.com/monsterhunterworld/mods/7015) - [Endemic Quality (Iceborne Edition) v1.4](https://www.nexusmods.com/monsterhunterworld/mods/2137) - [Remove censor bad word banned word filter v1.0](https://www.nexusmods.com/monsterhunterworld/mods/6956) - [Monster Weakness Icon Indicator for Iceborne (Hi-Res) vFinal](https://www.nexusmods.com/monsterhunterworld/mods/1938) - [Cuter Handler Face Model (Post-Iceborne) aka Make Handler Cuter Again v3.0](https://www.nexusmods.com/monsterhunterworld/mods/1914)

Additional Context

Plugin Repo

Fexty12573 commented 5 months ago

Good point. I never even thought about that. There is an existing system that goes over all fields in a plugin, and for any that implement IDisposable it calls Dispose on it. So for Task.Run, storing the resulting task as a field in your plugin should result in it being cleaned up. However this does not work for System.Thread.

So yes, perhaps having an OnUnload event like you suggested would be beneficial.

Regarding events being fired pre-OnLoad. Thanks for reporting that. I will change that so no events are called before OnLoad completes. (With the exception of OnPreMain and OnWinMain)

Fexty12573 commented 5 months ago

Actually, it seems calling Dispose on a Task does not terminate it. So that OnUnload event will be needed either way.

Fexty12573 commented 5 months ago

OnUnload event was implemented in aff54da73917d5b8a817a46d0be755cebb1a4dd1. Will close once 0.0.4 is out.