Sane: Rework non-deterministic ID handling

The new setup ensures each operation happens in a separate worker process, which is important as libusb on macOS seems to have some issues with running multiple SANE commands in a row.

NAPS2.Sdk/Platform/ISystemCompat.cs

@@ -44,6 +44,8 @@ internal interface ISystemCompat
 
     string SaneLibraryName { get; }
 
+    bool IsLibUsbReliable { get; }
+
     IntPtr LoadLibrary(string path);
 
     IntPtr LoadSymbol(IntPtr libraryHandle, string symbol);

NAPS2.Sdk/Platform/LinuxSystemCompat.cs

@@ -53,6 +53,8 @@ internal class LinuxSystemCompat : ISystemCompat
 
     public string SaneLibraryName => "libsane.so.1";
 
+    public bool IsLibUsbReliable => true;
+
     public IntPtr LoadLibrary(string path) => LinuxInterop.dlopen(path, RTLD_LAZY | RTLD_GLOBAL);
 
     public IntPtr LoadSymbol(IntPtr libraryHandle, string symbol) => LinuxInterop.dlsym(libraryHandle, symbol);

NAPS2.Sdk/Platform/MacSystemCompat.cs

@@ -64,6 +64,8 @@ public string[] LibrarySearchPaths
 
     public string SaneLibraryName => "libsane.1.dylib";
 
+    public bool IsLibUsbReliable => false;
+
     public IntPtr LoadLibrary(string path) => MacInterop.dlopen(path, RTLD_LAZY | RTLD_GLOBAL);
 
     public IntPtr LoadSymbol(IntPtr libraryHandle, string symbol) => MacInterop.dlsym(libraryHandle, symbol);

NAPS2.Sdk/Platform/WindowsSystemCompat.cs

@@ -47,6 +47,8 @@ internal abstract class WindowsSystemCompat : ISystemCompat
 
     public string SaneLibraryName => "sane.dll";
 
+    public bool IsLibUsbReliable => true;
+
     public IntPtr LoadLibrary(string path) => Win32.LoadLibrary(path);
 
     public string GetLoadError() => Marshal.GetLastWin32Error().ToString();

NAPS2.Sdk/Scan/Internal/Sane/Native/SaneClient.cs

@@ -11,7 +11,6 @@ private static SaneNativeLibrary GetNativeLibrary(ISaneInstallation saneInstalla
     {
         lock (SaneLock)
         {
-            saneInstallation.Initialize();
             return new SaneNativeLibrary(saneInstallation.LibraryPath, saneInstallation.LibraryDeps);
         }
     }

NAPS2.Sdk/Scan/Internal/Sane/SaneScanDriver.cs

@@ -9,7 +9,7 @@ namespace NAPS2.Scan.Internal.Sane;
 internal class SaneScanDriver : IScanDriver
 {
     private static string? _customConfigDir;
-    
+
     private readonly ScanningContext _scanningContext;
 
     public SaneScanDriver(ScanningContext scanningContext)
@@ -22,15 +22,6 @@ public SaneScanDriver(ScanningContext scanningContext)
             : File.Exists("/.flatpak-info")
                 ? new FlatpakSaneInstallation()
                 : new SystemSaneInstallation();
-        if (_customConfigDir == null && !OperatingSystem.IsWindows())
-        {
-            // SANE caches the SANE_CONFIG_DIR environment variable process-wide, which means that we can't willy-nilly
-            // change the config dir. However, if we use a static directory name and only create the actual directory
-            // when we want to use it, SANE will (without caching) use the directory when it exists, and fall back to
-            // the default config dir otherwise.
-            _customConfigDir = Path.Combine(_scanningContext.TempFolderPath, Path.GetRandomFileName());
-            Installation.SetCustomConfigDir(_customConfigDir);
-        }
 #else
         Installation = null!;
 #endif
@@ -53,20 +44,39 @@ void MaybeCallback(SaneDeviceInfo device)
 
         return Task.Run(() =>
         {
-            // TODO: This is crashing after a delay for no apparent reason.
-            // That's okay because we're in a worker process, but ideally we could fix it in SANE.
-            using var client = new SaneClient(Installation);
-            // TODO: We can use device.type and .vendor to help pick an icon etc.
-            // https://sane-project.gitlab.io/standard/api.html#device-descriptor-type
-            if (Installation.CanStreamDevices)
+            Installation.Initialize();
+            string? tempConfigDir = MaybeCreateTempConfigDirForSingleBackend(options.SaneOptions.Backend);
+            try
             {
-                client.StreamDevices(MaybeCallback, cancelToken);
+                // TODO: This is crashing after a delay for no apparent reason.
+                // That's okay because we're in a worker process, but ideally we could fix it in SANE.
+                using var client = new SaneClient(Installation);
+                // TODO: We can use device.type and .vendor to help pick an icon etc.
+                // https://sane-project.gitlab.io/standard/api.html#device-descriptor-type
+                if (Installation.CanStreamDevices)
+                {
+                    client.StreamDevices(MaybeCallback, cancelToken);
+                }
+                else
+                {
+                    foreach (var device in client.GetDevices())
+                    {
+                        MaybeCallback(device);
+                    }
+                }
             }
-            else
+            finally
             {
-                foreach (var device in client.GetDevices())
+                if (tempConfigDir != null)
                 {
-                    MaybeCallback(device);
+                    try
+                    {
+                        Directory.Delete(tempConfigDir, true);
+                    }
+                    catch (Exception ex)
+                    {
+                        _scanningContext.Logger.LogDebug(ex, "Error cleaning up temp SANE config dir");
+                    }
                 }
             }
         });
@@ -110,147 +120,101 @@ private static string GetName(SaneDeviceInfo device)
         return null;
     }
 
-    private static string GetBackend(string saneDeviceName)
-    {
-        return saneDeviceName.Split(':')[0];
-    }
+    public static string GetBackend(ScanDevice device) => GetBackend(device.ID);
+
+    private static string GetBackend(string saneDeviceName) => saneDeviceName.Split(':')[0];
 
     public Task Scan(ScanOptions options, CancellationToken cancelToken, IScanEvents scanEvents,
         Action<IMemoryImage> callback)
     {
         return Task.Run(() =>
         {
+            bool hasAtLeastOneImage = false;
             try
             {
-                ScanWithSaneDevice(options, cancelToken, scanEvents, callback, options.Device!.ID);
-            }
-            catch (DeviceOfflineException)
-            {
-                // Some SANE backends (e.g. airscan, genesys) have inconsistent IDs so "device offline" might actually
-                // just mean "device id has changed". We can query for a "backup" device that matches the name of the
-                // original device, and assume it's the same physical device, which should generally be correct.
-                string? backupDeviceId = QueryForBackupSaneDevice(options.Device!);
-                if (backupDeviceId == null)
-                {
-                    throw;
-                }
-                ScanWithSaneDevice(options, cancelToken, scanEvents, callback, backupDeviceId);
-            }
-        });
-    }
-
-    void ScanWithSaneDevice(ScanOptions options, CancellationToken cancelToken, IScanEvents scanEvents,
-        Action<IMemoryImage> callback, string deviceId)
-    {
-        bool hasAtLeastOneImage = false;
-        try
-        {
-            using var client = new SaneClient(Installation);
-            if (cancelToken.IsCancellationRequested) return;
-            _scanningContext.Logger.LogDebug("Opening SANE Device \"{ID}\"", deviceId);
-            using var device = client.OpenDevice(deviceId);
-            if (cancelToken.IsCancellationRequested) return;
-            var optionData = SetOptions(device, options);
-            var cancelOnce = new Once(device.Cancel);
-            cancelToken.Register(cancelOnce.Run);
-            try
-            {
-                if (!optionData.IsFeeder)
-                {
-                    var image = ScanPage(device, scanEvents, optionData) ??
-                                throw new DeviceException("SANE expected image");
-                    callback(image);
-                }
-                else
+                Installation.Initialize();
+                using var client = new SaneClient(Installation);
+                if (cancelToken.IsCancellationRequested) return;
+                _scanningContext.Logger.LogDebug("Opening SANE Device \"{ID}\"", options.Device!.ID);
+                using var device = client.OpenDevice(options.Device.ID);
+                if (cancelToken.IsCancellationRequested) return;
+                var optionData = SetOptions(device, options);
+                var cancelOnce = new Once(device.Cancel);
+                cancelToken.Register(cancelOnce.Run);
+                try
                 {
-                    while (ScanPage(device, scanEvents, optionData) is { } image)
+                    if (!optionData.IsFeeder)
                     {
-                        hasAtLeastOneImage = true;
+                        var image = ScanPage(device, scanEvents, optionData) ??
+                                    throw new DeviceException("SANE expected image");
                         callback(image);
                     }
-                }
-            }
-            finally
-            {
-                cancelOnce.Run();
-            }
-        }
-        catch (SaneException ex)
-        {
-            switch (ex.Status)
-            {
-                case SaneStatus.Good:
-                case SaneStatus.Cancelled:
-                    return;
-                case SaneStatus.NoDocs:
-                    if (!hasAtLeastOneImage)
+                    else
                     {
-                        throw new DeviceFeederEmptyException();
+                        while (ScanPage(device, scanEvents, optionData) is { } image)
+                        {
+                            hasAtLeastOneImage = true;
+                            callback(image);
+                        }
                     }
-
-                    break;
-                case SaneStatus.DeviceBusy:
-                    throw new DeviceBusyException();
-                case SaneStatus.Invalid:
-                    // TODO: Maybe not always correct? e.g. when setting options
-                    throw new DeviceOfflineException();
-                case SaneStatus.Jammed:
-                    throw new DevicePaperJamException();
-                case SaneStatus.CoverOpen:
-                    throw new DeviceCoverOpenException();
-                default:
-                    throw new DeviceException($"SANE error: {ex.Status}");
-            }
-        }
-    }
-
-    private string? QueryForBackupSaneDevice(ScanDevice device)
-    {
-        // If we couldn't get an ID match, we can call GetDevices again and see if we can find a name match.
-        // This can be very slow (10+ seconds) if we have to query every single backend (the normal SANE behavior for
-        // GetDevices). We can hack this by creating a temporary SANE config dir that only references the single backend
-        // we need, so it ends up being only ~1s.
-        _scanningContext.Logger.LogDebug(
-            "SANE Device appears offline; re-querying in case of ID change for name \"{Name}\"", device.Name);
-        string? tempConfigDir = MaybeCreateTempConfigDirForSingleBackend(GetBackend(device.ID));
-        try
-        {
-            using var client = new SaneClient(Installation);
-            var backupDevice = client.GetDevices()
-                .FirstOrDefault(deviceInfo => GetName(deviceInfo) == device.Name);
-            if (backupDevice.Name == null)
-            {
-                _scanningContext.Logger.LogDebug("No matching device found");
-                return null;
-            }
-            return backupDevice.Name;
-        }
-        finally
-        {
-            if (tempConfigDir != null)
-            {
-                try
+                }
+                finally
                 {
-                    Directory.Delete(tempConfigDir, true);
+                    cancelOnce.Run();
                 }
-                catch (Exception ex)
+            }
+            catch (SaneException ex)
+            {
+                switch (ex.Status)
                 {
-                    _scanningContext.Logger.LogDebug(ex, "Error cleaning up temp SANE config dir");
+                    case SaneStatus.Good:
+                    case SaneStatus.Cancelled:
+                        return;
+                    case SaneStatus.NoDocs:
+                        if (!hasAtLeastOneImage)
+                        {
+                            throw new DeviceFeederEmptyException();
+                        }
+
+                        break;
+                    case SaneStatus.DeviceBusy:
+                        throw new DeviceBusyException();
+                    case SaneStatus.Invalid:
+                        // TODO: Maybe not always correct? e.g. when setting options
+                        throw new DeviceOfflineException();
+                    case SaneStatus.Jammed:
+                        throw new DevicePaperJamException();
+                    case SaneStatus.CoverOpen:
+                        throw new DeviceCoverOpenException();
+                    default:
+                        throw new DeviceException($"SANE error: {ex.Status}");
                 }
             }
-        }
+        });
     }
 
-    private string? MaybeCreateTempConfigDirForSingleBackend(string backendName)
+    private string? MaybeCreateTempConfigDirForSingleBackend(string? backendName)
     {
-        if (!Directory.Exists(Installation.DefaultConfigDir) || _customConfigDir == null)
+        if (string.IsNullOrEmpty(backendName))
+        {
+            return null;
+        }
+        if (!Directory.Exists(Installation.DefaultConfigDir))
         {
             // Non-typical SANE installation where we don't know the config dir and can't do this optimization
             return null;
         }
-        // SANE normally doesn't provide a way to only query a single backend - it's all or nothing.
-        // However, there is a workaround - if we use the SANE_CONFIG_DIR environment variable, we can specify a custom
-        // config dir, which can have a dll.conf file that only has a single backend specified.
+        if (_customConfigDir == null)
+        {
+            // SANE caches the SANE_CONFIG_DIR environment variable process-wide, which means that we can't willy-nilly
+            // change the config dir. However, if we use a static directory name and only create the actual directory
+            // when we want to use it, SANE will (without caching) use the directory when it exists, and fall back to
+            // the default config dir otherwise.
+            _customConfigDir = Path.Combine(_scanningContext.TempFolderPath, Path.GetRandomFileName());
+            Installation.SetCustomConfigDir(_customConfigDir);
+        }
+        // By using a custom config dir with a dll.conf file that only has a single backend specified, we can force SANE
+        // to only check that backend
         Directory.CreateDirectory(_customConfigDir);
         // Copy the backend.conf file in case there's any important backend-specific configuration
         var backendConfFile = $"{backendName}.conf";
@@ -264,7 +228,7 @@ void ScanWithSaneDevice(ScanOptions options, CancellationToken cancelToken, ISca
         File.WriteAllText(Path.Combine(_customConfigDir, "dll.conf"), backendName);
         // Create an empty dll.d dir so SANE doesn't use the default one
         Directory.CreateDirectory(Path.Combine(_customConfigDir, "dll.d"));
-        _scanningContext.Logger.LogDebug("Create temp SANE config dir {Dir}", _customConfigDir);
+        _scanningContext.Logger.LogDebug("Created temp SANE config dir {Dir}", _customConfigDir);
         return _customConfigDir;
     }
 

NAPS2.Sdk/Scan/Internal/ScanOptionsValidator.cs

@@ -8,8 +8,7 @@ internal class ScanOptionsValidator
 {
     public ScanOptions ValidateAll(ScanOptions options, ScanningContext scanningContext, bool requireDevice)
     {
-        // Easy deep copy. Ideally we'd do this in a more efficient way.
-        options = options.ToXml().FromXml<ScanOptions>();
+        options = options.Clone();
 
         if (options.Device != null && options.Driver != Driver.Default)
         {

NAPS2.Sdk/Scan/SaneOptions.cs

@@ -5,4 +5,8 @@
 /// </summary>
 public class SaneOptions
 {
+    /// <summary>
+    /// Limit the devices queried by GetDevices/GetDeviceList to the given SANE backend.
+    /// </summary>
+    public string? Backend { get; set; }
 }