Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid duplicate concurrent transfer USB API calls #1104

Closed
emaxx-google opened this issue Oct 27, 2023 · 0 comments · Fixed by #1107
Closed

Avoid duplicate concurrent transfer USB API calls #1104

emaxx-google opened this issue Oct 27, 2023 · 0 comments · Fixed by #1107
Assignees

Comments

@emaxx-google
Copy link
Collaborator

Currently, every Libusb transfer made by the CCID driver results in a chrome.usb/WebUSB API call.

This is unnecessary and wasteful in some cases, because we might already have equivalent in-flight API calls for the previously canceled transfers. Besides consuming Chrome resources, these extra calls can reportedly lead to hitting the OS kernel limits on the number of active transfers, which completely breaks the USB communication.

Instead, we should enqueue the requests and make sure we only make one JS API call at a time (among all input transfers that read from the same endpoint).

@emaxx-google emaxx-google self-assigned this Oct 27, 2023
emaxx-google added a commit that referenced this issue Oct 27, 2023
When choosing which of active Libusb transfers should receive a
reply from the JS API, use the FIFO order rather than a random order.
Also change the internal structures so that FIFO is used uniformly,
which we'll use in follow-ups for correctly enqueueing JS API calls.

The FIFO order is more intuitive and better corresponds to Libusb
behavior on other platforms. Moreover, the FIFO property will be
crucial to avoid the situation when an enqueued JS API call is
discarded because the transfer happened to be resolved by the call
originally triggered by a different transfer.

This contributes to #462 and #1104.
emaxx-google added a commit that referenced this issue Oct 27, 2023
Move interrupt transfer replies in testing_smart_card_simulation
outside of the mutex.

This is needed for follow-ups that rework how our Libusb port
handles incoming requests, because the new implementation may
synchronously start a new USB API call when receiving the previous
call's result. Without the current change, this would deadlock in
the unit tests.

This change contributes to #1104.
emaxx-google added a commit that referenced this issue Oct 27, 2023
Set the stack size to 1 MiB in both Debug and Release modes.

Previously we were doing this only in Debug, and left the default
64 KiB otherwise, but with recent changes it turned out that we're
besides this (tough) limit. In the future we might want to
investigate whether something smaller than 1 MiB would make sense,
however for now WebAssembly builds aren't shipped anywhere, and we
just want to make the CI bots happy on follow-up changes.

This change contributes to #1104.
emaxx-google added a commit that referenced this issue Oct 27, 2023
Don't make new chrome.usb/WebUSB API calls in case there're already
running equivalent ones (for input transfers).

Besides consuming Chrome resources, these extra calls were reportedly
leading to hitting the OS kernel limits on the number of active
transfers, which completely broke the USB communication in some cases.

This fixes #1104.
emaxx-google added a commit that referenced this issue Nov 24, 2023
When choosing which of active Libusb transfers should receive a
reply from the JS API, use the FIFO order rather than a random order.
Also change the internal structures so that FIFO is used uniformly,
which we'll use in follow-ups for correctly enqueueing JS API calls.

The FIFO order is more intuitive and better corresponds to Libusb
behavior on other platforms. Moreover, the FIFO property will be
crucial to avoid the situation when an enqueued JS API call is
discarded because the transfer happened to be resolved by the call
originally triggered by a different transfer.

This contributes to #462 and #1104.
emaxx-google added a commit that referenced this issue Jan 18, 2024
Move interrupt transfer replies in testing_smart_card_simulation
outside of the mutex.

This is needed for follow-ups that rework how our Libusb port
handles incoming requests, because the new implementation may
synchronously start a new USB API call when receiving the previous
call's result. Without the current change, this would deadlock in
the unit tests.

This change contributes to #1104.
emaxx-google added a commit that referenced this issue Jan 18, 2024
Move interrupt transfer replies in testing_smart_card_simulation
outside of the mutex.

This is needed for follow-ups that rework how our Libusb port
handles incoming requests, because the new implementation may
synchronously start a new USB API call when receiving the previous
call's result. Without the current change, this would deadlock in
the unit tests.

This change contributes to #1104.
emaxx-google added a commit that referenced this issue Jan 18, 2024
Set the stack size to 1 MiB in both Debug and Release modes.

Previously we were doing this only in Debug, and left the default
64 KiB otherwise, but with recent changes it turned out that we're
besides this (tough) limit. In the future we might want to
investigate whether something smaller than 1 MiB would make sense,
however for now WebAssembly builds aren't shipped anywhere, and we
just want to make the CI bots happy on follow-up changes.

This change contributes to #1104.
emaxx-google added a commit that referenced this issue Jan 22, 2024
Set the stack size to 1 MiB in both Debug and Release modes.

Previously we were doing this only in Debug, and left the default
64 KiB otherwise, but with recent changes it turned out that we're
besides this (tough) limit. In the future we might want to
investigate whether something smaller than 1 MiB would make sense,
however for now WebAssembly builds aren't shipped anywhere, and we
just want to make the CI bots happy on follow-up changes.

This change contributes to #1104.
emaxx-google added a commit that referenced this issue Jan 22, 2024
Don't make new chrome.usb/WebUSB API calls in case there're already
running equivalent ones (for input transfers).

Besides consuming Chrome resources, these extra calls were reportedly
leading to hitting the OS kernel limits on the number of active
transfers, which completely broke the USB communication in some cases.

This fixes #1104.
emaxx-google added a commit that referenced this issue Jan 22, 2024
Don't make new chrome.usb/WebUSB API calls in case there're already
running equivalent ones (for input transfers).

Besides consuming Chrome resources, these extra calls were reportedly
leading to hitting the OS kernel limits on the number of active
transfers, which completely broke the USB communication in some cases.

This fixes #1104.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant