DeviceIoControl use async mode #1127

weishaodi2zss · 2022-04-13T12:07:10Z

DeviceIoControl maybe block in long time,we use async mode to control DeviceIoControl wait time.

mcuee · 2022-04-13T12:34:21Z

Please help to explain the background of this pull request and what is the issue you are trying to address.

What is the configuration of your system and how long is "long time" for your system?

weishaodi2zss · 2022-04-14T12:24:21Z

Please help to explain the background of this pull request and what is the issue you are trying to address.

What is the configuration of your system and how long is "long time" for your system?

we use libusb to get the bandwidth of the usb camera.In our application,our user report they will always block when call "DeviceIoControl " in the windows_winusb.c file. DeviceIoControl function execution keeps blocking.“long time” means forever.
Our application has more than 6000000 users，only fewer users have the DeviceIoControl Block problem.
configuration of out user's system:
windows 10
INTEL(R) CORE(TM) I5-8300H CPU @ 2.30GHZ

mcuee · 2022-04-14T12:33:15Z

Thanks for the updates. The user's system seem to be pretty decent and not a slow system.
Hopefully more reviewers can jump in to review this change to see if it has some side effects or not.

weishaodi2zss · 2022-04-18T06:37:07Z

Thanks for the updates. The user's system seem to be pretty decent and not a slow system. Hopefully more reviewers can jump in to review this change to see if it has some side effects or not.

How's the code review going ? Can you approve?

mcuee · 2022-04-18T11:50:13Z

How's the code review going ? Can you approve?

Unfortunately I am not a developer (mainly on testing and support side of libusb project) and I will not be able to review properly for more complex patch like this one.

And right now we do not have a real Windows developer to look into the patches, so you may have to be patient.

mcuee · 2022-04-28T02:51:04Z

Interestingly that this pull request seems to fix #1136.

Libusb deadlocks with the Nacon wired controller #1136

Updates: it seems to fix $1163 as well.

Windows: libusb_get_device_list deadlock with 8Bitdo Pro2 controller #1163

mcuee · 2022-05-08T08:23:16Z

@weishaodi2zss Please help to fix the warning that not all commits were signed. Thanks.

There is a warning from github.

Merging is blocked
The base branch requires all commits to be signed.

Edit: please ignore this request. It is not necessary.

weishaodi2zss · 2022-05-10T13:09:18Z

we find some side effects in our product,the commit will fix the side effect,please approve the pr,the pr have been applied our application which have more than 6000000 users and it work well in our application.

Thanks you for following my PR.
d8850e1

mcuee · 2022-05-10T13:41:08Z

@tormodvolden How do you like this pull request? I think it is probably okay to be merged.

tormodvolden · 2022-05-11T08:07:41Z

I don't understand enough of this to approve it. That there are still fixes coming in also means we shouldn't rush it. Why is the DeviceIoControl blocking indefinitely in the first place? I would be fine with including a workaround for a kernel driver issue, but the issue must then be fully understood and documented. I understand the patch uses overlapping I/O to implement a timeout, but e.g. why is 1s better than 10 s or 0.1s?

mcuee · 2022-07-14T02:45:21Z

@weishaodi2zss
Can you address the questions raised by @tormodvolden ? Thanks.

mcuee · 2023-01-23T10:57:36Z

@tormodvolden and @weishaodi2zss

Another possibility is to add an new option to libusb_set_option to choose to use async DeviceIoControl? How do you like this idea?

tormodvolden · 2023-01-27T12:35:44Z

our user report they will always block when call "DeviceIoControl " in the windows_winusb.c file

What is the libusb function causing this? EDIT: Looking at the suggest code change it seems to be the enumeration in libusb_init(). So which of these DeviceIoControl()'s are actually hanging? Maybe only one of them?

Youw · 2023-01-27T14:36:46Z

HIDAPI uses async DeviceIoControl - I can check this one out (a bit later).

add an new option to libusb_set_option to choose to use async DeviceIoControl?

I don't think that should be required.

Youw · 2023-01-27T14:39:33Z

Meanwhile, @weishaodi2zss do you mind fixing the conflict?

libusb/os/windows_winusb.c

weishaodi2zss · 2024-01-11T02:19:23Z

Take some time to continue reviewing the code here @Youw

Youw · 2024-01-16T14:42:14Z

libusb/os/windows_winusb.c

+
+	error = GetLastError();
+	if (error != ERROR_IO_PENDING) {
+		CancelIo(hDevice);


shouldn't this be:

Suggested change

CancelIo(hDevice);

CancelIoEx(hDevice, &overlapped);

?

Youw · 2024-01-16T14:42:24Z

libusb/os/windows_winusb.c

+
+	DWORD wret = WaitForSingleObject(event, IO_WAIT_TIME);
+	if (wret != WAIT_OBJECT_0) {
+		CancelIo(hDevice);


same as prev comment

Youw · 2024-01-16T14:43:42Z

libusb/os/windows_winusb.c

+		assert(bytes_written == *lpBytesReturned);
+
+		if (!ret) {
+			CancelIo(hDevice);


if we're here - WaitForSingleObject has returned the WAIT_OBJECT_0, which means there is no pending operations this function has started
probably best not to interfere with other IO operations that might be happening on the device

Suggested change

CancelIo(hDevice);

Youw · 2024-01-16T14:45:35Z

libusb/os/windows_winusb.c


 	priv = usbi_get_device_priv(dev);

 	// If the device is already initialized, we can stop here
 	if (priv->initialized)
 		return LIBUSB_SUCCESS;

+	io_event = CreateEvent(NULL, FALSE, FALSE, NULL);


explicitly use either CreateEventA or CreateEventW
to be consistent with other places, e.g. CreateFileA

Youw · 2024-01-16T14:47:01Z

libusb/os/windows_winusb.c


 	priv = usbi_get_device_priv(dev);

 	// If the device is already initialized, we can stop here
 	if (priv->initialized)
 		return LIBUSB_SUCCESS;

+	io_event = CreateEvent(NULL, FALSE, FALSE, NULL);
+	if (!io_event || io_event == INVALID_HANDLE_VALUE) return LIBUSB_ERROR_IO;


nit:

Suggested change

if (!io_event || io_event == INVALID_HANDLE_VALUE) return LIBUSB_ERROR_IO;

if (io_event == NULL || io_event == INVALID_HANDLE_VALUE) return LIBUSB_ERROR_IO;

Youw

Excuse me for long delays in the review. I'm having too much on my plate recently.

Functionally I think this is perfectly fine (except my latest comments).

@weishaodi2zss what is the best way to check this out? Maybe @mcuee (or someone else) could try it locally. I'm mostly conserned that all the positive scenarios should continue function as ther where (i.e. nothing breaks after this change).

mcuee · 2024-01-22T06:48:09Z

@weishaodi2zss

I'd like to carry out some tests on this. It will be good to sync up with git master first though.

mcuee · 2024-01-22T07:02:01Z

@weishaodi2zss

As of now, stress and stress_mt failed with an assertion fault, under my Dell Windows 10 system with external Type-C Docking Station.

$ ./tests/stress.exe
Starting test run: init_and_exit...
Success (0)
Starting test run: get_device_list...

$ ./tests/stress_mt.exe
Running multithreaded init/exit test...
Starting 8 threads
Running multithreaded init/exit test with enumeration...
Starting 8 threads
Assertion failed: bytes_written == *lpBytesReturned, file ../../libusb/os/windows_winusb.c, line 754
A

$ make check
Making check in libusb
make[1]: Entering directory '/c/work/libusb/libusb_pr1127/debug_clang/libusb'
make[1]: Nothing to be done for 'check'.
make[1]: Leaving directory '/c/work/libusb/libusb_pr1127/debug_clang/libusb'
Making check in examples
make[1]: Entering directory '/c/work/libusb/libusb_pr1127/debug_clang/examples'
make[1]: Nothing to be done for 'check'.
make[1]: Leaving directory '/c/work/libusb/libusb_pr1127/debug_clang/examples'
Making check in tests
make[1]: Entering directory '/c/work/libusb/libusb_pr1127/debug_clang/tests'
make  check-TESTS
make[2]: Entering directory '/c/work/libusb/libusb_pr1127/debug_clang/tests'
make[3]: Entering directory '/c/work/libusb/libusb_pr1127/debug_clang/tests'
FAIL: stress.exe
FAIL: stress_mt.exe
PASS: set_option.exe
PASS: init_context.exe
============================================================================
Testsuite summary for libusb-1.0 1.0.27-rc1
============================================================================
# TOTAL: 4
# PASS:  2
# SKIP:  0
# XFAIL: 0
# FAIL:  2
# XPASS: 0
# ERROR: 0
============================================================================
See tests/test-suite.log
Please report to libusb-devel@lists.sourceforge.net
============================================================================
make[3]: *** [Makefile:800: test-suite.log] Error 1
make[3]: Leaving directory '/c/work/libusb/libusb_pr1127/debug_clang/tests'
make[2]: *** [Makefile:908: check-TESTS] Error 2
make[2]: Leaving directory '/c/work/libusb/libusb_pr1127/debug_clang/tests'
make[1]: *** [Makefile:1015: check-am] Error 2
make[1]: Leaving directory '/c/work/libusb/libusb_pr1127/debug_clang/tests'
make: *** [Makefile:491: check-recursive] Error 1

$ cat tests/test-suite.log
=================================================
   libusb-1.0 1.0.27-rc1: tests/test-suite.log
=================================================

# TOTAL: 4
# PASS:  2
# SKIP:  0
# XFAIL: 0
# FAIL:  2
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: stress
============

Starting test run: init_and_exit...
Success (0)
Starting test run: get_device_list...
FAIL stress.exe (exit status: 127)

FAIL: stress_mt
===============

Assertion failed: bytes_written == *lpBytesReturned, file ../../libusb/os/windows_winusb.c, line 754
FAIL stress_mt.exe (exit status: 127)

Debug log for stress_mt.
stress_mt_log.txt

tormodvolden · 2024-01-22T08:10:05Z

I'd like to carry out some tests on this. It will be good to sync up with git master first though.

For testing you can also rebase this locally:

git fetch origin pull/1127/head
git checkout -b pr1127_rebase FETCH_HEAD
git rebase origin/master

Megamouse · 2024-01-23T00:01:04Z

This seems to kinda "fix" my issue in #1163
libusb_get_device_list does not block infinitely anymore.
Now it "only" takes ~15-20 seconds until I can continue.
I haven't tested anything else, since I don't actually own anything that needs libusb in our project .

I believe the reason why it takes so long is because with the faulty controller connected every single device runs into the timeout of 2 seconds for some reason.
Maybe the timed out async call has still an influence over the following calls ?
Of course it may just be some deeper upstream issue on windows...

mcuee · 2024-01-23T08:47:08Z

I'd like to carry out some tests on this. It will be good to sync up with git master first though.

For testing you can also rebase this locally:
git fetch origin pull/1127/head
git checkout -b pr1127_rebase FETCH_HEAD
git rebase origin/master

Thanks. Here is the new log, still the same issues.

CLANG64 /c/work/libusb/libusb_pr1127_new/debug_clang64
$ make check
Making check in libusb
make[1]: Entering directory '/c/work/libusb/libusb_pr1127_new/debug_clang64/libusb'
make[1]: Nothing to be done for 'check'.
make[1]: Leaving directory '/c/work/libusb/libusb_pr1127_new/debug_clang64/libusb'
Making check in examples
make[1]: Entering directory '/c/work/libusb/libusb_pr1127_new/debug_clang64/examples'
make[1]: Nothing to be done for 'check'.
make[1]: Leaving directory '/c/work/libusb/libusb_pr1127_new/debug_clang64/examples'
Making check in tests
make[1]: Entering directory '/c/work/libusb/libusb_pr1127_new/debug_clang64/tests'
make  check-TESTS
make[2]: Entering directory '/c/work/libusb/libusb_pr1127_new/debug_clang64/tests'
make[3]: Entering directory '/c/work/libusb/libusb_pr1127_new/debug_clang64/tests'
FAIL: stress.exe
FAIL: stress_mt.exe
PASS: set_option.exe
PASS: init_context.exe
============================================================================
Testsuite summary for libusb-1.0 1.0.27-rc1
============================================================================
# TOTAL: 4
# PASS:  2
# SKIP:  0
# XFAIL: 0
# FAIL:  2
# XPASS: 0
# ERROR: 0
============================================================================
See tests/test-suite.log
Please report to libusb-devel@lists.sourceforge.net
============================================================================
make[3]: *** [Makefile:800: test-suite.log] Error 1
make[3]: Leaving directory '/c/work/libusb/libusb_pr1127_new/debug_clang64/tests'
make[2]: *** [Makefile:908: check-TESTS] Error 2
make[2]: Leaving directory '/c/work/libusb/libusb_pr1127_new/debug_clang64/tests'
make[1]: *** [Makefile:1015: check-am] Error 2
make[1]: Leaving directory '/c/work/libusb/libusb_pr1127_new/debug_clang64/tests'
make: *** [Makefile:491: check-recursive] Error 1

$ cat tests/test-suite.log
=================================================
   libusb-1.0 1.0.27-rc1: tests/test-suite.log
=================================================

# TOTAL: 4
# PASS:  2
# SKIP:  0
# XFAIL: 0
# FAIL:  2
# XPASS: 0
# ERROR: 0

.. contents:: :depth: 2

FAIL: stress
============

Starting test run: init_and_exit...
Success (0)
Starting test run: get_device_list...
FAIL stress.exe (exit status: 127)

FAIL: stress_mt
===============

Assertion failed: bytes_written == *lpBytesReturned, file ../../libusb/os/windows_winusb.c, line 754
FAIL stress_mt.exe (exit status: 127)

mcuee · 2024-01-23T08:53:33Z

BTW, this has nothing to do with the external Dell Type C docking edition and external Ugreen USB Hub. Even with minimum setup (just the laptop, plus one USB mouse dongle and a headset), the issue is already there.

CLANG64 /c/work/libusb/libusb_pr1127_new/debug_clang64
$ ./examples/listdevs.exe
8087:0aaa (bus 1, device 5) path: 14
2717:5013 (bus 1, device 4) path: 3
8086:a36d (bus 1, device 0)
0a5c:5842 (bus 1, device 2) path: 10
8086:15db (bus 2, device 0)
047f:c056 (bus 1, device 19) path: 2
0bda:58fd (bus 1, device 6) path: 11

$ ./examples/testlibusb.exe
Dev (bus 1, device 5): 8087 - 0AAA speed: 12M
Dev (bus 1, device 4): 2717 - 5013 speed: 12M
  Manufacturer:              MI
  Product:                   Mi Wireless Mouse
Dev (bus 1, device 0): 8086 - A36D speed: 5G
Dev (bus 1, device 2): 0A5C - 5842 speed: 480M
Dev (bus 2, device 0): 8086 - 15DB speed: 5G
Dev (bus 1, device 19): 047F - C056 speed: 12M
  Manufacturer:              Plantronics
  Product:                   Plantronics Blackwire 3220 Series
Dev (bus 1, device 6): 0BDA - 58FD speed: 480M

$ ./tests/stress_mt.exe
Running multithreaded init/exit test...
Starting 8 threads
Running multithreaded init/exit test with enumeration...
Starting 8 threads
Assertion failed: bytes_written == *lpBytesReturned, file ../../libusb/os/windows_winusb.c, line 754
A

mcuee added the windows label Apr 13, 2022

tormodvolden mentioned this pull request Apr 25, 2022

Libusb deadlocks with the Nacon wired controller #1136

Open

mcuee mentioned this pull request Apr 28, 2022

Get device list takes an infinite amount of time with 3rd party controllers #804

Closed

ergo720 mentioned this pull request Jun 18, 2022

Updated libusb submodule to include deadlock fix Cxbx-Reloaded/Cxbx-Reloaded#2375

Merged

mcuee mentioned this pull request Jul 9, 2022

Windows: libusb_get_device_list deadlock with 8Bitdo Pro2 controller #1163

Closed

mcuee mentioned this pull request Oct 30, 2022

Misc: To release 1.0.27 #1094

Closed

tormodvolden reviewed Jan 27, 2023

View reviewed changes

libusb/os/windows_winusb.c Outdated Show resolved Hide resolved

tormodvolden mentioned this pull request Jan 27, 2023

libusb_get_device_list() slow on Windows #1237

Open

Youw self-requested a review February 15, 2023 21:58

sonatique mentioned this pull request Mar 5, 2023

Windows: Freezing in Kernel with libusb-win32 #1255

Open

weishaodi2zss closed this Dec 5, 2023

weishaodi2zss force-pushed the master branch from d8850e1 to 30a4434 Compare December 5, 2023 03:19

shaodiwei added 3 commits December 5, 2023 11:26

test

62a329f

revert

2b67e02

DeviceIoControl use async mode

62b4f05

weishaodi2zss reopened this Dec 5, 2023

Fixing Compilation Errors

30738ca

weishaodi2zss requested a review from tormodvolden December 7, 2023 06:11

Youw requested changes Dec 14, 2023

View reviewed changes

libusb/os/windows_winusb.c Outdated Show resolved Hide resolved

shaodiwei and others added 2 commits December 29, 2023 11:50

use one event.

5c1fc48

Merge branch 'libusb:master' into master

a6ff492

weishaodi2zss requested a review from Youw December 29, 2023 04:00

Youw reviewed Dec 29, 2023

View reviewed changes

libusb/os/windows_winusb.c Show resolved Hide resolved

libusb/os/windows_winusb.c Show resolved Hide resolved

fix all where io_event is not released

3763929

weishaodi2zss requested a review from Youw January 2, 2024 06:32

Youw reviewed Jan 16, 2024

View reviewed changes

tormodvolden mentioned this pull request Jan 31, 2024

Misc: To release 1.0.28 #1453

Open

mcuee mentioned this pull request Feb 15, 2024

Add support for SuperSpeed+ Capability Descriptors #1428

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DeviceIoControl use async mode #1127

DeviceIoControl use async mode #1127

weishaodi2zss commented Apr 13, 2022

mcuee commented Apr 13, 2022 •

edited

Loading

weishaodi2zss commented Apr 14, 2022 •

edited

Loading

mcuee commented Apr 14, 2022 •

edited

Loading

weishaodi2zss commented Apr 18, 2022

mcuee commented Apr 18, 2022

mcuee commented Apr 28, 2022 •

edited

Loading

mcuee commented May 8, 2022 •

edited

Loading

weishaodi2zss commented May 10, 2022 •

edited

Loading

mcuee commented May 10, 2022

tormodvolden commented May 11, 2022

mcuee commented Jul 14, 2022

mcuee commented Jan 23, 2023

tormodvolden commented Jan 27, 2023 •

edited

Loading

Youw commented Jan 27, 2023

Youw commented Jan 27, 2023

weishaodi2zss commented Jan 11, 2024

Youw Jan 16, 2024

Youw Jan 16, 2024

Youw Jan 16, 2024

Youw Jan 16, 2024 •

edited

Loading

Youw Jan 16, 2024

Youw left a comment

mcuee commented Jan 22, 2024

mcuee commented Jan 22, 2024 •

edited

Loading

tormodvolden commented Jan 22, 2024

Megamouse commented Jan 23, 2024

mcuee commented Jan 23, 2024

mcuee commented Jan 23, 2024

	if (!io_event \|\| io_event == INVALID_HANDLE_VALUE) return LIBUSB_ERROR_IO;
	if (io_event == NULL \|\| io_event == INVALID_HANDLE_VALUE) return LIBUSB_ERROR_IO;

DeviceIoControl use async mode #1127

Are you sure you want to change the base?

DeviceIoControl use async mode #1127

Conversation

weishaodi2zss commented Apr 13, 2022

mcuee commented Apr 13, 2022 • edited Loading

weishaodi2zss commented Apr 14, 2022 • edited Loading

mcuee commented Apr 14, 2022 • edited Loading

weishaodi2zss commented Apr 18, 2022

mcuee commented Apr 18, 2022

mcuee commented Apr 28, 2022 • edited Loading

mcuee commented May 8, 2022 • edited Loading

weishaodi2zss commented May 10, 2022 • edited Loading

mcuee commented May 10, 2022

tormodvolden commented May 11, 2022

mcuee commented Jul 14, 2022

mcuee commented Jan 23, 2023

tormodvolden commented Jan 27, 2023 • edited Loading

Youw commented Jan 27, 2023

Youw commented Jan 27, 2023

weishaodi2zss commented Jan 11, 2024

Youw Jan 16, 2024

Choose a reason for hiding this comment

Youw Jan 16, 2024

Choose a reason for hiding this comment

Youw Jan 16, 2024

Choose a reason for hiding this comment

Youw Jan 16, 2024 • edited Loading

Choose a reason for hiding this comment

Youw Jan 16, 2024

Choose a reason for hiding this comment

Youw left a comment

Choose a reason for hiding this comment

mcuee commented Jan 22, 2024

mcuee commented Jan 22, 2024 • edited Loading

tormodvolden commented Jan 22, 2024

Megamouse commented Jan 23, 2024

mcuee commented Jan 23, 2024

mcuee commented Jan 23, 2024

mcuee commented Apr 13, 2022 •

edited

Loading

weishaodi2zss commented Apr 14, 2022 •

edited

Loading

mcuee commented Apr 14, 2022 •

edited

Loading

mcuee commented Apr 28, 2022 •

edited

Loading

mcuee commented May 8, 2022 •

edited

Loading

weishaodi2zss commented May 10, 2022 •

edited

Loading

tormodvolden commented Jan 27, 2023 •

edited

Loading

Youw Jan 16, 2024 •

edited

Loading

mcuee commented Jan 22, 2024 •

edited

Loading