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 linux BLE deadlock #26418

Merged
merged 7 commits into from
May 11, 2023
Merged

Conversation

tehampson
Copy link
Contributor

@tehampson tehampson commented May 8, 2023

Problem:

Solution:

  • The requirement can be that ChipDeviceScanner::StartScan is called with the ChipStackLock. These seems to allow both cases to coexist.
  • The destructor calling CancelTimer still does seem to be dependent on if it was called in scheduled work that does call TimerExpiredCallback, or some other thread. For the time being acquiring the ChipStackLock logic over there still makes sense.

Test:

  • chip-tool pairing ble-wifi 1 asdf asdf 20202021 3840, no longer deadlocks
  • In chip-repl issuing asdf = chip.ble.DiscoverSync(timeoutMs=2000) followed by for x in asdf: print(f"TMsg {x}") works.
  • In chip-repl, issuing devCtrl.CommissionWiFi(3840, 20202021, 1, "asdf", "asdf") no longer deadlocks.

tehampson referenced this pull request May 8, 2023
…is started (#26338)

* chip-repl hits the Code is unsafe/racy assert when BLE commissioning is started

* Add lock/unlock to CancelTimer

* Fix a few other issue

* Restyle

* Small fix

* Last few fixes
@github-actions
Copy link

github-actions bot commented May 8, 2023

PR #26418: Size comparison from 6c0c5ea to f1c4ab1

Full report (1 build for cc32xx)
platform target config section 6c0c5ea f1c4ab1 change % change
cc32xx lock CC3235SF_LAUNCHXL 0 0 0 0.0
(read only) 605090 605090 0 0.0
(read/write) 204164 204164 0 0.0
.ARM.attributes 44 44 0 0.0
.ARM.exidx 8 8 0 0.0
.bss 197576 197576 0 0.0
.comment 206 206 0 0.0
.data 1468 1468 0 0.0
.debug_abbrev 957621 957621 0 0.0
.debug_aranges 101136 101136 0 0.0
.debug_frame 341512 341512 0 0.0
.debug_info 19609755 19609755 0 0.0
.debug_line 2666632 2666632 0 0.0
.debug_line_str 513 513 0 0.0
.debug_loc 33340 33340 0 0.0
.debug_loclists 1489225 1489225 0 0.0
.debug_ranges 4984 4984 0 0.0
.debug_rnglists 94315 94315 0 0.0
.debug_str 3108982 3108982 0 0.0
.ramVecs 780 780 0 0.0
.resetVecs 64 64 0 0.0
.rodata 104354 104354 0 0.0
.shstrtab 265 265 0 0.0
.stack 2048 2048 0 0.0
.strtab 483384 483384 0 0.0
.symtab 287328 287328 0 0.0
.text 498612 498612 0 0.0

@tehampson tehampson changed the title Rely on checking if stack lock is already aquired before acquiring Fix linux BLE deadload May 10, 2023
@tehampson tehampson marked this pull request as ready for review May 10, 2023 19:53
Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

Thank you for hunting this down!

src/controller/python/chip/ble/LinuxImpl.cpp Show resolved Hide resolved
@tehampson tehampson enabled auto-merge (squash) May 11, 2023 14:39
@tehampson tehampson merged commit d55c592 into project-chip:master May 11, 2023
@bzbarsky-apple bzbarsky-apple changed the title Fix linux BLE deadload Fix linux BLE deadlock May 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants