fix: properly fire serial-port-added and serial-port-removed events - #34958
Merged
Conversation
codebytere
reviewed
Jul 19, 2022
codebytere
left a comment
codebytere
left a comment
Member
There was a problem hiding this comment.
It look like the tests themselves pass but then in the dtor there's a crash:
Received signal 11 <unknown> 000000000000
0 Electron Framework 0x0000000123eb0952 base::debug::CollectStackTrace(void**, unsigned long) + 18
1 Electron Framework 0x0000000123dcd413 base::debug::StackTrace::StackTrace() + 19
2 Electron Framework 0x0000000123eb08a1 base::debug::(anonymous namespace)::StackDumpSignalHandler(int, __siginfo*, void*) + 1345
3 libsystem_platform.dylib 0x00007ff803414dfd _sigtramp + 29
4 Electron Framework 0x0000000125dbcb25 mojo::core::ports::Node::AcceptEvent(mojo::core::ports::NodeName const&, std::Cr::unique_ptr<mojo::core::ports::Event, std::Cr::default_delete<mojo::core::ports::Event>>) + 2277
5 Electron Framework 0x0000000123e22b21 base::ScopedValidateSequenceChecker::ScopedValidateSequenceChecker(base::SequenceCheckerImpl const&) + 33
6 Electron Framework 0x0000000123df0e97 base::internal::WeakReference::IsValid() const + 39
7 Electron Framework 0x000000011f3e6dc8 base::ObserverList<content::SerialDelegate::Observer, false, true, base::internal::CheckedObserverAdapter>::RemoveObserver(content::SerialDelegate::Observer const*) + 104
8 Electron Framework 0x000000011f3e90d7 electron::SerialChooserController::~SerialChooserController() + 327
9 Electron Framework 0x000000011f3e566c electron::ElectronSerialDelegate::~ElectronSerialDelegate() + 124
10 Electron Framework 0x000000011f37dbce electron::ElectronBrowserClient::~ElectronBrowserClient() + 190
11 Electron Framework 0x000000011f37dc6e electron::ElectronBrowserClient::~ElectronBrowserClient() + 14
12 Electron Framework 0x000000011f27c850 ElectronMain + 192
13 dyld 0x000000010966651e start + 462
[end of stack trace]
✗ Electron tests failed with kill signal SIGSEGV.
codebytere
reviewed
Jul 19, 2022
Based on 2309652: [webhid] Notify chooser context observers on shutdown | https://chromium-review.googlesource.com/c/chromium/src/+/2309652
jkleinsc
force-pushed
the
fix-serial-device-changed-events
branch
from
July 21, 2022 18:13
c4a1a66 to
dfdb74d
Compare
Member
Author
|
The crash was resolved by following an approach similar to: [webhid] Notify chooser context observers on shutdown. |
ckerr
approved these changes
Jul 21, 2022
codebytere
approved these changes
Jul 25, 2022
Member
Author
|
Merging as CI failure is known CI issue unrelated to this PR. |
|
Release Notes Persisted
|
Contributor
|
I was unable to backport this PR to "17-x-y" cleanly; |
Contributor
|
I was unable to backport this PR to "18-x-y" cleanly; |
Contributor
|
I was unable to backport this PR to "19-x-y" cleanly; |
Contributor
|
I have automatically backported this PR to "20-x-y", please check out #35047 |
ghost
mentioned this pull request
schetle
pushed a commit
to schetle/electron
that referenced
this pull request
Nov 3, 2022
…lectron#34958) Based on 2309652: [webhid] Notify chooser context observers on shutdown | https://chromium-review.googlesource.com/c/chromium/src/+/2309652
khalwa
pushed a commit
to solarwindscloud/electron
that referenced
this pull request
Feb 22, 2023
…lectron#34958) Based on 2309652: [webhid] Notify chooser context observers on shutdown | https://chromium-review.googlesource.com/c/chromium/src/+/2309652
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Change
Closes #34887.
Checklist
npm testpassesRelease Notes
Notes: fixed
serial-port-addedandserial-port-removedevents not firing.