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 some text entry leaks on Darwin. #21286

Merged

Conversation

bzbarsky-apple
Copy link
Contributor

We can get OnNewInterface twice for the same interface. When the happens we
replace the old InterfaceInfo with a new one in the map. Because our cleanup
was from ~ResolveContext, that meant we leaked any allocated memory the
InterfaceInfo was holding on to.

The fix is to make InterfaceInfo manage its own memory and make sure we transfer
the ownership properly via move constructors when inserting into the map.

Problem

See above.

Change overview

See above.

Testing

Ran darwin-framework-tool pairing code with LeakSanitizer and observed that the leak that had detected without these changes is gone.

We can get OnNewInterface twice for the same interface.  When the happens we
replace the old InterfaceInfo with a new one in the map.  Because our cleanup
was from ~ResolveContext, that meant we leaked any allocated memory the
InterfaceInfo was holding on to.

The fix is to make InterfaceInfo manage its own memory and make sure we transfer
the ownership properly via move constructors when inserting into the map.
@github-actions
Copy link

github-actions bot commented Jul 27, 2022

PR #21286: Size comparison from 5cd92fb to df101d8

Increases (5 builds for bl602, cc13x2_26x2, nrfconnect, telink)
platform target config section 5cd92fb df101d8b change % change
bl602 lighting-app bl602+rpc (read/write) 1426978 1426986 8 0.0
.text 1083236 1083240 4 0.0
cc13x2_26x2 pump-app LP_CC2652R7 (read only) 681047 681055 8 0.0
.text 591388 591396 8 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 text 808780 808784 4 0.0
telink light-switch-app tlsr9518adk80d (read/write) 799684 799692 8 0.0
text 567216 567218 2 0.0
lighting-app tlsr9518adk80d text 583788 583790 2 0.0
Decreases (3 builds for bl602, cc13x2_26x2, efr32)
platform target config section 5cd92fb df101d8b change % change
bl602 lighting-app bl602 .text 1051572 1051568 -4 -0.0
cc13x2_26x2 pump-app LP_CC2652R7 (read/write) 161336 161328 -8 -0.0
efr32 lock-app BRD4161A+wf200 (read/write) 1128384 1128368 -16 -0.0
.text 981948 981932 -16 -0.0
Full report (32 builds for bl602, cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
platform target config section 5cd92fb df101d8b change % change
bl602 lighting-app bl602 (read/write) 1381578 1381578 0 0.0
.bss 117610 117610 0 0.0
.data 4480 4480 0 0.0
.text 1051572 1051568 -4 -0.0
bl602+rpc (read/write) 1426978 1426986 8 0.0
.bss 125050 125050 0 0.0
.data 4600 4600 0 0.0
.text 1083236 1083240 4 0.0
cc13x2_26x2 all-clusters-app LP_CC2652R7 (read only) 668475 668475 0 0.0
(read/write) 182884 182884 0 0.0
.bss 74252 74252 0 0.0
.data 3356 3356 0 0.0
.rodata 88411 88411 0 0.0
.text 579748 579748 0 0.0
all-clusters-minimal-app LP_CC2652R7 (read only) 634059 634059 0 0.0
(read/write) 157820 157820 0 0.0
.bss 73548 73548 0 0.0
.data 3356 3356 0 0.0
.rodata 77635 77635 0 0.0
.text 556100 556100 0 0.0
lock-ftd LP_CC2652R7 (read only) 671607 671607 0 0.0
(read/write) 169944 169944 0 0.0
.bss 71332 71332 0 0.0
.data 3280 3280 0 0.0
.rodata 76463 76463 0 0.0
.text 594664 594664 0 0.0
lock-mtd LP_CC2652R7 (read only) 653835 653835 0 0.0
(read/write) 183404 183404 0 0.0
.bss 67020 67020 0 0.0
.data 3280 3280 0 0.0
.rodata 101163 101163 0 0.0
.text 552192 552192 0 0.0
pump-app LP_CC2652R7 (read only) 681047 681055 8 0.0
(read/write) 161336 161328 -8 -0.0
.bss 71396 71396 0 0.0
.data 3280 3280 0 0.0
.rodata 89175 89175 0 0.0
.text 591388 591396 8 0.0
pump-controller-app LP_CC2652R7 (read only) 666815 666815 0 0.0
(read/write) 175704 175704 0 0.0
.bss 71532 71532 0 0.0
.data 3276 3276 0 0.0
.rodata 85007 85007 0 0.0
.text 581328 581328 0 0.0
shell LP_CC2652R7 (read only) 660942 660942 0 0.0
(read/write) 185936 185936 0 0.0
.bss 76572 76572 0 0.0
.data 3360 3360 0 0.0
.rodata 85174 85174 0 0.0
.text 575452 575452 0 0.0
cyw30739 light cyw930739m2evb_01 (read/write) 583358 583358 0 0.0
.app_xip_area 460176 460176 0 0.0
.bss 65640 65640 0 0.0
.data 728 728 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
lock cyw930739m2evb_01 (read/write) 589278 589278 0 0.0
.app_xip_area 461368 461368 0 0.0
.bss 70368 70368 0 0.0
.data 732 732 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
ota-requestor-no-progress-logging cyw930739m2evb_01 (read/write) 589138 589138 0 0.0
.app_xip_area 466772 466772 0 0.0
.bss 64880 64880 0 0.0
.data 672 672 0 0.0
.rodata 0 0 0 0.0
.text 112 112 0 0.0
efr32 lighting-app BRD4161A (read/write) 1088208 1088208 0 0.0
.bss 133276 133276 0 0.0
.data 2048 2048 0 0.0
.text 952864 952864 0 0.0
BRD4161A+rpc (read/write) 1142500 1142500 0 0.0
.bss 149956 149956 0 0.0
.data 2260 2260 0 0.0
.text 990264 990264 0 0.0
BRD4161A+rs911x (read/write) 952880 952880 0 0.0
.bss 140992 140992 0 0.0
.data 2048 2048 0 0.0
.text 809820 809820 0 0.0
lock-app BRD4161A+wf200 (read/write) 1128384 1128368 -16 -0.0
.bss 144360 144360 0 0.0
.data 2056 2056 0 0.0
.text 981948 981932 -16 -0.0
window-app BRD4161A (read/write) 1081684 1081684 0 0.0
.bss 134748 134748 0 0.0
.data 2076 2076 0 0.0
.text 944836 944836 0 0.0
esp32 all-clusters-app c3devkit (read only) 1022468 1022468 0 0.0
(read/write) 1486578 1486578 0 0.0
.dram0.bss 70288 70288 0 0.0
.dram0.data 14600 14600 0 0.0
.flash.rodata 216256 216256 0 0.0
.flash.text 1022468 1022468 0 0.0
.iram0.text 62902 62902 0 0.0
m5stack (read only) 1076139 1076139 0 0.0
(read/write) 488600 488600 0 0.0
.dram0.bss 75800 75800 0 0.0
.dram0.data 34144 34144 0 0.0
.flash.rodata 246660 246660 0 0.0
.flash.text 1070755 1070755 0 0.0
.iram0.text 123267 123267 0 0.0
k32w light k32w0+release (read/write) 641808 641808 0 0.0
.bss 69728 69728 0 0.0
.data 2028 2028 0 0.0
.text 567324 567324 0 0.0
lock k32w0+release (read/write) 699104 699104 0 0.0
.bss 70168 70168 0 0.0
.data 2036 2036 0 0.0
.text 624172 624172 0 0.0
linux chip-tool-ipv6only arm64 (read only) 9836724 9836724 0 0.0
(read/write) 678593 678593 0 0.0
.bss 32897 32897 0 0.0
.data 3272 3272 0 0.0
.data.rel.ro 623936 623936 0 0.0
.dynamic 560 560 0 0.0
.got 13536 13536 0 0.0
.init 24 24 0 0.0
.init_array 192 192 0 0.0
.rodata 467940 467940 0 0.0
.text 7786484 7786484 0 0.0
thermostat-no-ble arm64 (read only) 2342868 2342868 0 0.0
(read/write) 141585 141585 0 0.0
.bss 55297 55297 0 0.0
.data 1672 1672 0 0.0
.data.rel.ro 75856 75856 0 0.0
.dynamic 560 560 0 0.0
.got 4992 4992 0 0.0
.init 24 24 0 0.0
.init_array 408 408 0 0.0
.rodata 139180 139180 0 0.0
.text 1966464 1966464 0 0.0
mbed lock-app CY8CPROTO_062_4343W+release (read only) 6224 6224 0 0.0
(read/write) 2449392 2449392 0 0.0
.bss 214508 214508 0 0.0
.data 5872 5872 0 0.0
.text 1412036 1412036 0 0.0
nrfconnect all-clusters-app nrf52840dk_nrf52840 (read/write) 1172767 1172767 0 0.0
bss 143132 143132 0 0.0
rodata 141936 141936 0 0.0
text 808780 808784 4 0.0
all-clusters-minimal-app nrf52840dk_nrf52840 (read/write) 1152819 1152819 0 0.0
bss 142368 142368 0 0.0
rodata 133468 133468 0 0.0
text 798092 798092 0 0.0
p6 all-clusters-app default (read only) 881568 881568 0 0.0
(read/write) 1687092 1687092 0 0.0
.bss 149128 149128 0 0.0
.data 2648 2648 0 0.0
.text 1526928 1526928 0 0.0
all-clusters-minimal-app default (read only) 882288 882288 0 0.0
(read/write) 1631196 1631196 0 0.0
.bss 148408 148408 0 0.0
.data 2648 2648 0 0.0
.text 1471752 1471752 0 0.0
light-app default (read only) 890592 890592 0 0.0
(read/write) 1551548 1551548 0 0.0
.bss 140312 140312 0 0.0
.data 2440 2440 0 0.0
.text 1400408 1400408 0 0.0
lock-app default (read only) 886120 886120 0 0.0
(read/write) 1589148 1589148 0 0.0
.bss 144768 144768 0 0.0
.data 2456 2456 0 0.0
.text 1433536 1433536 0 0.0
telink light-switch-app tlsr9518adk80d (read/write) 799684 799692 8 0.0
bss 70808 70808 0 0.0
noinit 40416 40416 0 0.0
text 567216 567218 2 0.0
lighting-app tlsr9518adk80d (read/write) 819792 819792 0 0.0
bss 71652 71652 0 0.0
noinit 40416 40416 0 0.0
text 583788 583790 2 0.0

@woody-apple woody-apple enabled auto-merge (squash) July 27, 2022 23:30
@woody-apple woody-apple merged commit e4916bb into project-chip:master Jul 27, 2022
github-actions bot pushed a commit that referenced this pull request Jul 28, 2022
We can get OnNewInterface twice for the same interface.  When the happens we
replace the old InterfaceInfo with a new one in the map.  Because our cleanup
was from ~ResolveContext, that meant we leaked any allocated memory the
InterfaceInfo was holding on to.

The fix is to make InterfaceInfo manage its own memory and make sure we transfer
the ownership properly via move constructors when inserting into the map.
@bzbarsky-apple bzbarsky-apple deleted the fix-darwin-text-entry-leak branch July 28, 2022 01:49
woody-apple added a commit that referenced this pull request Jul 29, 2022
We can get OnNewInterface twice for the same interface.  When the happens we
replace the old InterfaceInfo with a new one in the map.  Because our cleanup
was from ~ResolveContext, that meant we leaked any allocated memory the
InterfaceInfo was holding on to.

The fix is to make InterfaceInfo manage its own memory and make sure we transfer
the ownership properly via move constructors when inserting into the map.

Co-authored-by: Boris Zbarsky <[email protected]>
isiu-apple pushed a commit to isiu-apple/connectedhomeip that referenced this pull request Sep 16, 2022
We can get OnNewInterface twice for the same interface.  When the happens we
replace the old InterfaceInfo with a new one in the map.  Because our cleanup
was from ~ResolveContext, that meant we leaked any allocated memory the
InterfaceInfo was holding on to.

The fix is to make InterfaceInfo manage its own memory and make sure we transfer
the ownership properly via move constructors when inserting into the map.
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.

2 participants