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

fix(app-shell): monitor usb only once #15047

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

sfoster1
Copy link
Member

We've observed a crash in the app node side that occurs inside usb-detection likely to do with some surprisingly-shaped descriptors, and those descriptors are polled only (a) on request or (b) when you initialize a webusb device. So let's remove the webusb device initialization and see if that helps.

Also, the webusb connect listeners are as far as I can tell completely duplicative of the listeners in system-info/usb-devices, I wonder if this was a merge conflict?

Closes RQA-2641

We've observed a crash in the app node side that occurs inside
usb-detection likely to do with some surprisingly-shaped descriptors,
and those descriptors are polled only (a) on request or (b) when you
initialize a webusb device. So let's remove the webusb device
initialization and see if that helps.

Also, the webusb connect listeners are as far as I can tell completely
duplicative of the listeners in system-info/usb-devices, I wonder if
this was a merge conflict?

Closes RQA-2641
@sfoster1 sfoster1 requested a review from a team as a code owner April 30, 2024 15:08
@sfoster1 sfoster1 requested review from TamarZanzouri, koji and y3rsh and removed request for a team April 30, 2024 15:08
Copy link
Contributor

@koji koji left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

Copy link
Contributor

@TamarZanzouri TamarZanzouri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Collaborator

@y3rsh y3rsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool so now just handled in
app-shell/src/system-info/index.ts

@sfoster1
Copy link
Member Author

Cool so now just handled in app-shell/src/system-info/index.ts

and always was. I think this code would have been leaking listeners and causing connect events to fire a bunch of times.

@sfoster1 sfoster1 merged commit aa4a835 into edge Apr 30, 2024
33 checks passed
@sfoster1 sfoster1 deleted the rqa-2641-dont-double-usb-monitor-app-build-both branch April 30, 2024 16:57
Carlos-fernandez pushed a commit that referenced this pull request May 20, 2024
We've observed a crash in the app node side that occurs inside
usb-detection likely to do with some surprisingly-shaped descriptors,
and those descriptors are polled only (a) on request or (b) when you
initialize a webusb device. So let's remove the webusb device
initialization and see if that helps.

Also, the webusb connect listeners are as far as I can tell completely
duplicative of the listeners in system-info/usb-devices, I wonder if
this was a merge conflict?

Closes RQA-2641
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants