Rem0o / FanControl.DellPlugin

Plugin for https://github.com/Rem0o/FanControl.Releases that provides support for Dell laptops using https://github.com/AaronKelley/DellFanManagement.
39 stars 8 forks source link

[Bug] Loading the dell driver doesn't check for existing running instances of the dell driver #5

Closed JamesYeoman closed 2 years ago

JamesYeoman commented 3 years ago

I don't know what causes it, but for some reason, on system startup, the dell driver gets loaded before fan control, which causes the dell plugin to fail to start because it tries to start the dell driver (which is already running). This then results in an inability to use the plugin because the sys file is locked by a process that I don't have access to

JamesYeoman commented 3 years ago

So I've found the actual issue: DellPlugin tries to copy the sys file from the Plugins folder to the fan control root folder, but because for some reason the sys file in the fan control root folder is already loaded by the Service Control Manager, it fails, which results in the dell driver handle not being created.

Found at this line, the assumption that the file can be overwritten is the cause of the inconsistent start-up race condition failure.

Error from the logfile

System.IO.IOException: The process cannot access the file '<fan control install location>\bzh_dell_smm_io_x64.sys' because it is being used by another process.
   at System.IO.__Error.WinIOError(Int32 errorCode, String maybeFullPath)
   at System.IO.File.InternalCopy(String sourceFileName, String destFileName, Boolean overwrite, Boolean checkHost)
   at System.IO.FileInfo.CopyTo(String destFileName, Boolean overwrite)
   at FanControl.DellPlugin.DellPlugin.Initialize()
   at FanControl.Domain.ComputerAccessLayer.Initialize(Nullable`1 disableStorage)

I still don't know where or how the driver is getting loaded from to cause the race condition, but a proposed solution is as follows:

if(driverFileExistsInFanControlDirectory())
{
    if(serviceControlManagerHasDriverLoaded())
    {
        if(driverInFanControlDirectoryIsOlderThanDriverInPluginsFolder())
        {
            unloadDriverFromServiceControlManager();
            replaceDriverWithNewerFile();
        }
    }
}

The functions are just for illustrative purposes, and the actual logic would likely be done inline rather than in a function, but the general idea is there

JamesYeoman commented 3 years ago

Ok, so, according to SysInternals' Process Explorer, bzh_dell_smm_io_x64.sys is loaded by the System process, and it has an autostart registry entry image

Rem0o commented 2 years ago

the sys file being locked up should be fixed by fancontrol handling closing sessions better now.