Use ExchangeHandle to track ref count of ExchangeContext

[kghost]

Problem

#7124
#6978
#7012
#6918

This PR is going to fix ExchangeContext life-cycle management problem once for all

Change overview

Introduce a shared_ptr like ExchangeHandle to automatically track the ref counter of ExchangeContext to replace raw pointer of ExchangeContext.

Testing

Not yet, make it draft first.

[github-actions]

Size increase report for "nrfconnect-example-build" from ebd9e0a

File	Section	File	VM
chip-shell.elf	text	500	500
chip-shell.elf	device_handles	-4	-4
chip-lock.elf	text	1340	1340
chip-lock.elf	rodata	104	104
chip-lock.elf	bss	0	36
chip-lock.elf	init_array	8	8
chip-lock.elf	[LOAD #3 [RW]]	0	-4
chip-lock.elf	device_handles	-4	-4

Full report output

BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_info,0,63833
.debug_line,0,6354
.debug_abbrev,0,3993
.debug_str,0,648
.debug_ranges,0,584
text,500,500
.symtab,0,144
.strtab,0,118
.debug_frame,0,96
.debug_aranges,0,40
.shstrtab,0,-2
device_handles,-4,-4
.debug_loc,0,-1060

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

sections,vmsize,filesize
.debug_info,0,101695
.debug_str,0,16052
.debug_line,0,10386
.debug_abbrev,0,5383
.debug_loc,0,3992
.debug_ranges,0,2192
.strtab,0,1484
text,1340,1340
.symtab,0,656
.debug_frame,0,316
.debug_aranges,0,144
rodata,104,104
bss,36,0
init_array,8,8
[LOAD #3 [RW]],-4,0
device_handles,-4,-4

[github-actions]

Size increase report for "gn_qpg6100-example-build" from ebd9e0a

File	Section	File	VM
chip-qpg6100-lighting-example.out	.text	1320	1320
chip-qpg6100-lighting-example.out	.bss	0	40
chip-qpg6100-lighting-example.out	.data	8	8
chip-qpg6100-lighting-example.out	.heap	0	-48

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
.debug_info,0,94777
.debug_str,0,16097
.debug_line,0,10262
.debug_abbrev,0,5116
.debug_loc,0,2696
.debug_ranges,0,2032
.text,1320,1320
.strtab,0,1163
.symtab,0,576
.debug_frame,0,316
.debug_aranges,0,136
.bss,40,0
.data,8,8
.shstrtab,0,-3
.heap,-48,0
[Unmapped],0,-1320

[bzbarsky-apple]

For what it's worth, I am currently working on a redesign of exchange context lifetime semantics that aims to fix these same issues.

Just making things shared_ptr-like does not actually fix all the lifetime problems we have, as far as I can tell. We might want to have some sort of handle setup in addition to the semantic changes; still sorting that out.

Importantly, we need to make it much harder to get the Close() semantics wrong, which this PR doesn't really address.

[kghost]

IMHO, ExchangeContext::Close() will only initiate graceful shutdown, and ExchangeContext::Abort() will force put the exchange in an error state, and release underlying resources (timers, etc). Both of them should not affect life-cycle of the object.

[bzbarsky-apple]

I am probably OK with decoupling Close() and the reference counting. But one of our major lifetime issues is "someone is holding on to this for too long", and having a shared_ptr doesn't, on its own, help with that. What helps is knowing when you actually need to hold a reference.

So to be clear, doing this sort of thing may well make sense, but is not enough on its own. To actually fix the issues we are running into with exchange lifetime we need, imo, some more general changes to how exchange lifetime is managed. I am trying to get those changes together in the next few days; we should just coordinate the changes because they are touching a lot of the same code.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale]

This stale pull request has been automatically closed. Thank you for your contributions.