[Bug 58962] New: WINE-TWAIN: multiple loading of twain_32.dll spoils in memory variables
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: wine-bugs(a)list.winehq.org Reporter: lenar.shakirov(a)gmail.com 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. */ -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58962 --- Comment #1 from Lenar Shakirov <lenar.shakirov(a)gmail.com> --- https://github.com/snejok/wine/commit/7f21f2e5fa49d8d8419f4090d1e25b34222f81... -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
http://bugs.winehq.org/show_bug.cgi?id=58962 Bernd Herd <codeberg(a)herdsoft.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |codeberg(a)herdsoft.com --- Comment #2 from Bernd Herd <codeberg(a)herdsoft.com> --- I've just filed a merge request that adds saving the data source selected by the user to the registry. You might want to check if this fills your need: https://gitlab.winehq.org/wine/wine/-/merge_requests/9731 I'd see the cause of what you've observed here in the twain_32.dll (which is now calling twaindsm.dll). The uninitialized data for the data source version has been addressed recently in commit https://gitlab.winehq.org/wine/wine/-/commit/5b50d54a7d2bcb2435c2868c7a443db... That memset in dlls/twaindsm/dsm_ctrl.c:80 was added to initialize those non-initialized variables. Maybe the sane.ds should set some non-zero values for version, maybe the wine version? But before the change, the value was uninitialized and thus more or less random. The fact that the get_identity code is called 4 times when there are 3 scanners is part of the design: It is called from dsm_ctrl.c until it repeats itself in line 94, so there must be always one call more than there are configured scanners, just to detect the repitition. That method of having more that one value for TW_IDENTITY from one .DS data source is not part of the Twain specification. I guess it was introduced for sane.ds and gphoto2.ds. The reason that unloading / realoading the twain_32.dll alias twaindsm.dll does not reset the iterator is also in the twaindsm source code: https://gitlab.winehq.org/wine/wine/-/blob/master/dlls/twaindsm/dsm_ctrl.c?r... /* This causes crashes due to still open Windows, so leave out for now. * FreeLibrary (currentDS->hmod); */ That means the twaindsm.dll does not free the sane.ds when the DS gets closed because there is still this workaround for a bug already addressed in this commit: https://gitlab.winehq.org/wine/wine/-/commit/46305670a3488770d73077f7b074705... If the sane.ds would get unloaded, the iterator would be reset on every reload of twain_32.dll / sane.ds. However I'd not currently want to remove this obsolete workaround while wine is in a code freeze state. -- Do not reply to this email, post in Bugzilla using the above URL to reply. You are receiving this mail because: You are watching all bug changes.
participants (1)
-
WineHQ Bugzilla