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.

Sign up for GitHub

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

Jump to bottom

Add mDNS shutdown #9621

Merged
merged 1 commit into from
Sep 13, 2021
Merged

Add mDNS shutdown #9621

merged 1 commit into from
Sep 13, 2021

Conversation

kghost
Copy link
Contributor

@kghost kghost commented Sep 11, 2021 •
edited
Loading

Problem

Hit a CI problem due to that udp endpoint in mDNS is used after udp pool has been freed.

Change overview

Add shutdown of mDNS service, shutdown the mDNS server and close udp endpoint before release udp endpoint pool

Testing

Verified using unit-tests

Triggered a CI failure in PR #9590

WARNING: ThreadSanitizer: heap-use-after-free (pid=29980)
  Read of size 8 at 0x7b0c000020c0 by main thread (mutexes: write M79):
    #0 std::_Rb_tree<chip::Inet::UDPEndPoint*, chip::Inet::UDPEndPoint*, std::_Identity<chip::Inet::UDPEndPoint*>, std::less<chip::Inet::UDPEndPoint*>, std::allocator<chip::Inet::UDPEndPoint*> >::_M_lower_bound(std::_Rb_tree_node<chip::Inet::UDPEndPoint*>*, std::_Rb_tree_node_base*, chip::Inet::UDPEndPoint* const&) /usr/include/c++/9/bits/stl_tree.h:1929 (chip-tool+0x341066)
    #1 std::_Rb_tree<chip::Inet::UDPEndPoint*, chip::Inet::UDPEndPoint*, std::_Identity<chip::Inet::UDPEndPoint*>, std::less<chip::Inet::UDPEndPoint*>, std::allocator<chip::Inet::UDPEndPoint*> >::find(chip::Inet::UDPEndPoint* const&) /usr/include/c++/9/bits/stl_tree.h:2557 (chip-tool+0x340e83)
    #2 std::set<chip::Inet::UDPEndPoint*, std::less<chip::Inet::UDPEndPoint*>, std::allocator<chip::Inet::UDPEndPoint*> >::find(chip::Inet::UDPEndPoint* const&) /usr/include/c++/9/bits/stl_set.h:795 (chip-tool+0x340d04)
    #3 chip::System::ObjectPoolHeap<chip::Inet::UDPEndPoint, 32u>::ReleaseObject(chip::Inet::UDPEndPoint*) ../../../examples/chip-tool/third_party/connectedhomeip/src/system/SystemPoolHeap.h:55 (chip-tool+0x340b70)
    #4 chip::Inet::UDPEndPointDeletor::Release(chip::Inet::UDPEndPoint*) ../../../examples/chip-tool/third_party/connectedhomeip/src/inet/UDPEndPoint.h:113 (chip-tool+0x33fd67)
    #5 chip::AtomicReferenceCounted<chip::Inet::UDPEndPoint, chip::Inet::UDPEndPointDeletor, 1>::Release() ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/core/ReferenceCounted.h:115 (chip-tool+0x340cb8)
    #6 chip::Inet::UDPEndPoint::Free() ../../../examples/chip-tool/third_party/connectedhomeip/src/inet/UDPEndPoint.cpp:437 (chip-tool+0x3405e5)
    #7 ShutdownEndpoint ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/mdns/minimal/Server.cpp:118 (chip-tool+0x2fa127)
    #8 mdns::Minimal::ServerBase::Shutdown() ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/mdns/minimal/Server.cpp:135 (chip-tool+0x2fa28a)
    #9 mdns::Minimal::ServerBase::~ServerBase() ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/mdns/minimal/Server.cpp:126 (chip-tool+0x2fa195)
    #10 mdns::Minimal::Server<30ul>::~Server() ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/mdns/minimal/Server.h:161 (chip-tool+0x349aa7)
    #11 chip::Mdns::GlobalMinimalMdnsServer::~GlobalMinimalMdnsServer() ../../../examples/chip-tool/third_party/connectedhomeip/src/lib/mdns/MinimalMdnsServer.h:69 (chip-tool+0x349d0d)
    #12 cxa_at_exit_wrapper ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:393 (libtsan.so.0+0x2bc24)

  Previous write of size 8 at 0x7b0c000020c0 by main thread:
    #0 operator delete(void*) ../../../../src/libsanitizer/tsan/tsan_new_delete.cpp:126 (libtsan.so.0+0x8b2c8)
    #1 __gnu_cxx::new_allocator<std::_Rb_tree_node<chip::Inet::UDPEndPoint*> >::deallocate(std::_Rb_tree_node<chip::Inet::UDPEndPoint*>*, unsigned long) /usr/include/c++/9/ext/new_allocator.h:128 (chip-tool+0x33e1f1)
    #2 std::allocator_traits<std::allocator<std::_Rb_tree_node<chip::Inet::UDPEndPoint*> > >::deallocate(std::allocator<std::_Rb_tree_node<chip::Inet::UDPEndPoint*> >&, std::_Rb_tree_node<chip::Inet::UDPEndPoint*>*, unsigned long) /usr/include/c++/9/bits/alloc_traits.h:470 (chip-tool+0x33e002)
    #3 std::_Rb_tree<chip::Inet::UDPEndPoint*, chip::Inet::UDPEndPoint*, std::_Identity<chip::Inet::UDPEndPoint*>, std::less<chip::Inet::UDPEndPoint*>, std::allocator<chip::Inet::UDPEndPoint*> >::_M_put_node(std::_Rb_tree_node<chip::Inet::UDPEndPoint*>*) /usr/include/c++/9/bits/stl_tree.h:584 (chip-tool+0x33d98c)
    #4 std::_Rb_tree<chip::Inet::UDPEndPoint*, chip::Inet::UDPEndPoint*, std::_Identity<chip::Inet::UDPEndPoint*>, std::less<chip::Inet::UDPEndPoint*>, std::allocator<chip::Inet::UDPEndPoint*> >::_M_drop_node(std::_Rb_tree_node<chip::Inet::UDPEndPoint*>*) /usr/include/c++/9/bits/stl_tree.h:651 (chip-tool+0x33cb99)
    #5 std::_Rb_tree<chip::Inet::UDPEndPoint*, chip::Inet::UDPEndPoint*, std::_Identity<chip::Inet::UDPEndPoint*>, std::less<chip::Inet::UDPEndPoint*>, std::allocator<chip::Inet::UDPEndPoint*> >::_M_erase(std::_Rb_tree_node<chip::Inet::UDPEndPoint*>*) /usr/include/c++/9/bits/stl_tree.h:1915 (chip-tool+0x33c0af)
    #6 std::_Rb_tree<chip::Inet::UDPEndPoint*, chip::Inet::UDPEndPoint*, std::_Identity<chip::Inet::UDPEndPoint*>, std::less<chip::Inet::UDPEndPoint*>, std::allocator<chip::Inet::UDPEndPoint*> >::~_Rb_tree() /usr/include/c++/9/bits/stl_tree.h:995 (chip-tool+0x33b83d)
    #7 std::set<chip::Inet::UDPEndPoint*, std::less<chip::Inet::UDPEndPoint*>, std::allocator<chip::Inet::UDPEndPoint*> >::~set() /usr/include/c++/9/bits/stl_set.h:281 (chip-tool+0x33b167)
    #8 chip::System::ObjectPoolHeap<chip::Inet::UDPEndPoint, 32u>::~ObjectPoolHeap() ../../../examples/chip-tool/third_party/connectedhomeip/src/system/SystemPoolHeap.h:37 (chip-tool+0x3412a9)
    #9 cxa_at_exit_wrapper ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:393 (libtsan.so.0+0x2bc24)

@restyled-io restyled-io bot mentioned this pull request Sep 11, 2021
Closed
@github-actions
Copy link

Size increase report for "gn_qpg-example-build" from 5a9c063

File Section File VM
chip-qpg6100-lighting-example.out .text 16 16
Full report output 
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-qpg6100-lighting-example.out.map and ./pull_artifact/chip-qpg6100-lighting-example.out.map:

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: unknown file type for file './pull_artifact/chip-qpg6100-lighting-example.out.map'

Comparing ./master_artifact/chip-qpg6100-lighting-example.out and ./pull_artifact/chip-qpg6100-lighting-example.out:

sections,vmsize,filesize
.strtab,0,153
.debug_info,0,136
.debug_str,0,114
.symtab,0,80
.debug_abbrev,0,47
.debug_line,0,38
.debug_frame,0,32
.debug_aranges,0,16
.debug_ranges,0,16
.text,16,16
.debug_loc,0,5
.shstrtab,0,-1
[Unmapped],0,-16

@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 5a9c063

File Section File VM
chip-lock.elf text 16 16
chip-lock.elf rodata 8 8
chip-shell.elf text 12 12
chip-shell.elf rodata 8 8
chip-shell.elf device_handles -12 -12
Full report output 
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.strtab,0,153
.debug_info,0,136
.debug_loc,0,131
.debug_str,0,117
.symtab,0,80
.debug_abbrev,0,47
.debug_line,0,41
.debug_frame,0,32
.debug_aranges,0,16
.debug_ranges,0,16
text,16,16
rodata,8,8
.shstrtab,0,3

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_info,0,173
.strtab,0,153
.debug_str,0,117
.symtab,0,80
.debug_abbrev,0,79
.debug_line,0,40
.debug_frame,0,32
.debug_loc,0,23
.debug_aranges,0,16
.debug_ranges,0,16
text,12,12
rodata,8,8
.shstrtab,0,-1
device_handles,-12,-12

@github-actions
Copy link

Size increase report for "esp32-example-build" from 5a9c063

File Section File VM
chip-ipv6only-app.elf .flash.text -172 -172
chip-shell.elf .flash.text 27 32
Full report output 
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-persistent-storage.elf and ./pull_artifact/chip-persistent-storage.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-pigweed-app.elf and ./pull_artifact/chip-pigweed-app.elf:

sections,vmsize,filesize

Comparing ./master_artifact/chip-ipv6only-app.elf and ./pull_artifact/chip-ipv6only-app.elf:

sections,vmsize,filesize
.flash.text,-172,-172
[Unmapped],0,-3924

Comparing ./master_artifact/chip-lock-app.elf and ./pull_artifact/chip-lock-app.elf:

sections,vmsize,filesize
.debug_info,0,107
.debug_str,0,70
.debug_line,0,62
.debug_frame,0,24
.debug_aranges,0,8
.debug_ranges,0,8
.debug_loc,0,-3

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
[Unmapped],0,4069
.debug_info,0,268
.debug_str,0,130
.debug_line,0,122
.strtab,0,122
.debug_abbrev,0,64
.debug_frame,0,48
.flash.text,32,27
.symtab,0,32
.debug_aranges,0,16
.debug_ranges,0,16
.shstrtab,0,2
.debug_loc,0,-12

Comparing ./master_artifact/chip-bridge-app.elf and ./pull_artifact/chip-bridge-app.elf:

sections,vmsize,filesize
.debug_info,0,107
.debug_str,0,74
.debug_line,0,62
.debug_frame,0,24
.debug_aranges,0,8
.debug_ranges,0,8
.debug_loc,0,-3

Comparing ./master_artifact/chip-temperature-measurement-app.elf and ./pull_artifact/chip-temperature-measurement-app.elf:

sections,vmsize,filesize
.debug_info,0,107
.debug_str,0,74
.debug_line,0,62
.debug_frame,0,24
.debug_aranges,0,8
.debug_ranges,0,8
.debug_loc,0,5

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,110
.debug_str,0,72
.debug_loc,0,67
.debug_line,0,62
.debug_frame,0,28
.debug_abbrev,0,15
.debug_aranges,0,8
.debug_ranges,0,8
.riscv.attributes,0,2

@msandstedt
Copy link
Contributor

At first glance, appears to fix the same (or similar) problem as was fixed for avahi with #9108.

@andy31415 andy31415 merged commit 3da26cf into project-chip:master Sep 13, 2021
@bzbarsky-apple
Copy link
Contributor

This was basically fixing #8349, right?

andy31415 added a commit to andy31415/connectedhomeip that referenced this pull request Sep 13, 2021
@andy31415
Revert "Add mDNS shutdown (project-chip#9621)"
871867f 
This reverts commit 3da26cf.

Tizen builds seem broken since this change:

```
2021-09-13 15:22:31 INFO
/__w/connectedhomeip/connectedhomeip/out/tizen-arm-light/../../examples/lighting-app/linux/third_party/connectedhomeip/src/lib/mdns/Discovery_ImplPlatform.h:46:
undefined reference to `chip::Mdns::ChipMdnsShutdown()'
409
2021-09-13 15:22:31 INFO    collect2: error: ld returned 1 exit status
```
andy31415 added a commit that referenced this pull request Sep 13, 2021
@andy31415
Revert "Add mDNS shutdown (#9621)" (#9640)
92d3a28 
This reverts commit 3da26cf.

Tizen builds seem broken since this change:

```
2021-09-13 15:22:31 INFO
/__w/connectedhomeip/connectedhomeip/out/tizen-arm-light/../../examples/lighting-app/linux/third_party/connectedhomeip/src/lib/mdns/Discovery_ImplPlatform.h:46:
undefined reference to `chip::Mdns::ChipMdnsShutdown()'
409
2021-09-13 15:22:31 INFO    collect2: error: ld returned 1 exit status
```
@kghost kghost deleted the mdns-shutdown branch September 14, 2021 02:24
kghost added a commit to kghost/connectedhomeip that referenced this pull request Sep 14, 2021
@kghost
Add mDNS shutdown (project-chip#9621)
95e5aab
@kghost kghost mentioned this pull request Sep 14, 2021
Merged
kghost added a commit to kghost/connectedhomeip that referenced this pull request Sep 14, 2021
@kghost
Add mDNS shutdown (project-chip#9621)
37f6841
kghost added a commit to kghost/connectedhomeip that referenced this pull request Sep 14, 2021
@kghost
Add mDNS shutdown (project-chip#9621)
158a0a5
andy31415 pushed a commit that referenced this pull request Sep 14, 2021
@kghost
Add mDNS shutdown (#9621) (#9674)
54b88e7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andy31415 andy31415 andy31415 approved these changes

@msandstedt msandstedt msandstedt approved these changes

@saurabhst saurabhst saurabhst approved these changes

@Damian-Nordic Damian-Nordic Damian-Nordic approved these changes

@bzbarsky-apple bzbarsky-apple Awaiting requested review from bzbarsky-apple

@chrisdecenzo chrisdecenzo Awaiting requested review from chrisdecenzo

@hawk248 hawk248 Awaiting requested review from hawk248

@jepenven-silabs jepenven-silabs Awaiting requested review from jepenven-silabs

Assignees
No one assigned
Labels
controller lib platform review - approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kghost @msandstedt @bzbarsky-apple @andy31415 @saurabhst @Damian-Nordic