KSP-ModularManagement / KSPe

Extensions and utilities for Kerbal Space Program
http://ksp.lisias.net/add-ons/KSPAPIExtensions
Other
11 stars 6 forks source link

Mono's Process.Process is leaking File Handlers when using `using` #29

Closed Lisias closed 2 years ago

Lisias commented 2 years ago

I'm running many KSP versions as test beds on a MacMini 6.2 under MacOS Mojave.

And I got this nasty issue to handle this weekend: now and then, I started to get a Win32Exception came from nowhere at KSP Startup on a heavily modded test bed.

Digging on the origin, I found the code being borked is my Shell.Command utility, and it makes use of Process.Process inside an using, as dictates the developer's good manners and sanitary habits.

But, yet, that freaking Win32Exceptions kept jumping on my KSP.log.

By removing one of my add'ons using KSPe.Light, the problem gone away. By putting it back, it came back - so it's deterministic on my rig.

I know this (bad) smell, I had it before (I'm not a kid).

Then I hacked a interim version of KSPe.Light for the add'on I used on the previous test, switching the using clausule by a try…finally and explicitly using Close() from the Process.Process object. The problem vanished.

By putting back the original KSPe.Light from the original distribution zip, the problem came back.

(sigh).

Oukey, problem diagnosed.

Lisias commented 2 years ago

Digging around, I came to this link: https://stackoverflow.com/questions/33803017/what-are-the-differences-between-process-close-and-process-dispose

I'm guessing that this code:

/// <internalonly/>
    /// <devdoc>
    ///    <para>
    ///       Free any resources associated with this component.
    ///    </para>
    /// </devdoc>
    protected override void Dispose(bool disposing) {
        if( !disposed) {
            if (disposing) {
                //Dispose managed and unmanaged resources
                Close();
            }
            this.disposed = true;
            base.Dispose(disposing);                
        }            
    }

is not exactly what's being used on Unity's Mono - or perhaps they screwed up the disposing or the this.disposed thingies somehow.

(sigh)

Lisias commented 2 years ago

In a way or another, I had to register this crap somewhere, so a GitHub issue appears to be the best place.

The fix was implemented on commit https://github.com/net-lisias-ksp/KSPe/commit/facda5baab9e22ef9e1f4c496968c385c52e1cac and published on the 2.4.1.14 Pre Release.