[Bug 58962] New: WINE-TWAIN: multiple loading of twain_32.dll spoils in memory variables

[WineHQ Bugzilla]

http://bugs.winehq.org/show_bug.cgi?id=58962

Bug ID: 58962
           Summary: WINE-TWAIN: multiple loading of twain_32.dll spoils in
                    memory variables
           Product: Wine
           Version: 10.14
          Hardware: x86-64
                OS: Linux
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: -unknown
          Assignee: [e-mail]
          Reporter: [e-mail]
      Distribution: ---

Created attachment 79663
  --> http://bugs.winehq.org/attachment.cgi?id=79663
wine-dlls-twain_32-tests-dsm_c.patch

I have a problem with my scan utility that can use WIA or TWAIN interactively
and loads twain_32.dll on the fly.

Problem reproduced when 2 or more scanners used (sane-devices)

It's looks like that:
1. loads twain_32.dll on the start, when it needs list of devices
2. loads twain_32.dll every time when try to scan
3. on the first scan I get my first scanner
4. on the second scan I get my first scanner
5. on the third scan I get my second scanner
6. on the four scan I get my first scanner again

You can reproduce it by 3 steps:
1. Configure any 2 scanners, for example virtual sane-test and sane-pnm:
a) echo -e 'pnm\ntest' > /etc/sane.d/dll.conf
b) sed -i 's/number_of_devices 2/number_of_devices 1/' /etc/sane.d/test.conf
c) scanimage -L
device `test:0' is a Noname frontend-tester virtual device
device `pnm:0' is a Noname PNM file reader virtual device

2. Now you need to patch wine/dlls/twain_32/tests/dsm.c by simple patch
wine-dlls-twain_32-tests-dsm_c.patch , build this test and get
i386-windows/twain_32_test.exe

3. Now when you run:

WINETEST_INTERACTIVE=1 wine /tmp/twain_32_test.exe &>/tmp/twain32.log

You get twain32.log, that contains interesting lines:

grep -a Manufacturer /tmp/twain32.log

dsm.c:779: [Scanner 1|Version 0.0()|Protocol 1.8|SupportedGroups
0x40000003|Manufacturer Noname|Family frontend-tester|ProductName test:0]
dsm.c:779: [Scanner 2|Version 0.0()|Protocol 1.8|SupportedGroups
0x40000003|Manufacturer Noname|Family PNM file reader|ProductName pnm:0]
dsm.c:779: [Scanner 1|Version 64380.17(t)|Protocol 1.8|SupportedGroups
0x40000003|Manufacturer Noname|Family frontend-tester|ProductName test:0]
dsm.c:779: [Scanner 2|Version 64380.17(t)|Protocol 1.8|SupportedGroups
0x40000003|Manufacturer Noname|Family PNM file reader|ProductName pnm:0]
dsm.c:779: [Scanner 1|Version 64380.17(t)|Protocol 1.8|SupportedGroups
0x40000003|Manufacturer Noname|Family PNM file reader|ProductName pnm:0]
dsm.c:779: [Scanner 2|Version 64380.17(t)|Protocol 1.8|SupportedGroups
0x40000003|Manufacturer Noname|Family frontend-tester|ProductName test:0]
dsm.c:779: [Scanner 1|Version 64380.17(l)|Protocol 1.8|SupportedGroups
0x40000003|Manufacturer Noname|Family frontend-tester|ProductName test:0]
dsm.c:779: [Scanner 2|Version 64380.17(l)|Protocol 1.8|SupportedGroups
0x40000003|Manufacturer Noname|Family PNM file reader|ProductName pnm:0]

As you can see, there is 2 problems:

1. On first and second twain_32.dll loading there is a "Family frontend-tester"
on first place, but on third loading "Family PNM file reader" moved on first
place

2. "Version" was '0.0()' on first loading, but changed to "64380.17(t)" after
that

For me I fix this behavior by changing logic of get_identity() from sane.ds:

--- a/wine/dlls/sane.ds/unixlib.c
+++ b/wine/dlls/sane.ds/unixlib.c
@@ -213,7 +213,15 @@ static NTSTATUS get_identity( void *args )
     static int cur_dev;

detect_sane_devices();
-    if (!device_list[cur_dev]) return STATUS_DEVICE_NOT_CONNECTED;
+
+    if (!device_list[cur_dev] || !device_list[cur_dev]->model ||
+        !device_list[cur_dev]->vendor ||
+        !device_list[cur_dev]->name)
+    {
+        cur_dev = 0; /* wrap to begin */
+        return STATUS_DEVICE_NOT_CONNECTED;
+    }
+
     id->ProtocolMajor = TWON_PROTOCOLMAJOR;
     id->ProtocolMinor = TWON_PROTOCOLMINOR;
     id->SupportedGroups = DG_CONTROL | DG_IMAGE | DF_DS2;
@@ -222,11 +230,6 @@ static NTSTATUS get_identity( void *args )
     lstrcpynA (id->ProductFamily, device_list[cur_dev]->model,
sizeof(id->ProductFamily) - 1);
     cur_dev++;

-    if (!device_list[cur_dev] || !device_list[cur_dev]->model  ||
-        !device_list[cur_dev]->vendor ||
-       !device_list[cur_dev]->name)
-       cur_dev = 0; /* wrap to begin */
-
     return STATUS_SUCCESS;
 }

Current get_identity() implementation increments cur_dev more than necessary,
this is especially noticeable when there are 2 (or more scanners) and
twain_32.dll loads multiple times.

Because of this, twain_add_onedriver() function from dlls/twain_32/dsm_ctrl.c
adds 3 device instead of 2

Everything works fine when twain_32.dll loads one time, but multiple loading of
twain_32.dll spoils in memory variables

I don't really sure that it good idea to return "not success code", because as
I understand TWAIN specification awaiting only "success code" from
DG_CONTROL/DAT_IDENTITY/MSG_GET

https://github.com/twain/twain-specification/blob/master/versions/2.4/TWAIN-...

Page 7-60

DG_CONTROL / DAT_IDENTITY / MSG_GET (from Source Manager to Source)

Return Codes:
TWRC_SUCCESS /* This operation must succeed. */