[darwin] Race when BleConnection stop is called #25522

vivien-apple · 2023-03-07T12:42:44Z

Problem

There is a data race when BleConnection::Stop is called from CancelConnection. The underlying objects are accessed by both the chip work queue and the ble work queue.

I have found that while working on #25374

github-actions · 2023-03-07T13:09:47Z

PR #25522: Size comparison from 68185d3 to 289f772

Increases (1 build for cc32xx)

platform	target	config	section	`68185d3`	`289f772`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL	.debug_info	20267066	20267068	2	0.0

Full report (3 builds for cc32xx, qpg)

platform	target	config	section	`68185d3`	`289f772`	change
cc32xx	lock	CC3235SF_LAUNCHXL		0	0	0
			(read only)	644425	644425	0
			(read/write)	203688	203688	0
			.ARM.attributes	44	44	0
			.ARM.exidx	8	8	0
			.bss	197088	197088	0
			.comment	194	194	0
			.data	1480	1480	0
			.debug_abbrev	930235	930235	0
			.debug_aranges	87336	87336	0
			.debug_frame	300028	300028	0
			.debug_info	20267066	20267068	2
			.debug_line	2659698	2659698	0
			.debug_loc	2802749	2802749	0
			.debug_ranges	282952	282952	0
			.debug_str	3023892	3023892	0
			.ramVecs	780	780	0
			.resetVecs	64	64	0
			.rodata	105929	105929	0
			.shstrtab	232	232	0
			.stab	204	204	0
			.stabstr	441	441	0
			.stack	2048	2048	0
			.strtab	378514	378514	0
			.symtab	256624	256624	0
			.text	536372	536372	0
qpg	lighting-app	qpg6105+debug	(read/write)	1152020	1152020	0
			.bss	99812	99812	0
			.data	852	852	0
			.text	599116	599116	0
	lock-app	qpg6105+debug	(read/write)	1119084	1119084	0
			.bss	96292	96292	0
			.data	864	864	0
			.text	566184	566184	0

src/platform/Darwin/BleConnectionDelegateImpl.mm

bzbarsky-apple

Nice, thank you!

github-actions · 2023-03-07T18:53:06Z

PR #25522: Size comparison from 4203370 to cc9d3e3

Full report (5 builds for cc32xx, linux, qpg)

platform	target	config	section	`4203370`	`cc9d3e3`
cc32xx	lock	CC3235SF_LAUNCHXL		0	0
			(read only)	644425	644425
			(read/write)	203688	203688
			.ARM.attributes	44	44
			.ARM.exidx	8	8
			.bss	197088	197088
			.comment	194	194
			.data	1480	1480
			.debug_abbrev	930235	930235
			.debug_aranges	87336	87336
			.debug_frame	300028	300028
			.debug_info	`2026706`	`2026706`
			.debug_line	2659698	2659698
			.debug_loc	2802749	2802749
			.debug_ranges	282952	282952
			.debug_str	3023892	3023892
			.ramVecs	780	780
			.resetVecs	64	64
			.rodata	105929	105929
			.shstrtab	232	232
			.stab	204	204
			.stabstr	441	441
			.stack	2048	2048
			.strtab	378514	378514
			.symtab	256624	256624
			.text	536372	536372
linux	chip-tool-ipv6only	arm64	(read only)	12156404	12156404
			(read/write)	733448	733448
			.bss	34296	34296
			.data	3008	3008
			.data.rel.ro	675552	675552
			.dynamic	560	560
			.got	15376	15376
			.init	24	24
			.init_array	216	216
			.rodata	588308	588308
			.text	9827476	9827476
	thermostat-no-ble	arm64	(read only)	2520900	2520900
			(read/write)	145224	145224
			.bss	56344	56344
			.data	1784	1784
			.data.rel.ro	77688	77688
			.dynamic	560	560
			.got	5360	5360
			.init	24	24
			.init_array	432	432
			.rodata	150800	150800
			.text	`2107712`	`2107712`
qpg	lighting-app	qpg6105+debug	(read/write)	1152020	1152020
			.bss	99812	99812
			.data	852	852
			.text	599116	599116
	lock-app	qpg6105+debug	(read/write)	1119084	1119084
			.bss	96292	96292
			.data	864	864
			.text	566184	566184

vivien-apple · 2023-03-08T14:05:46Z

@bzbarsky-apple Can you have a look at the latest change ? I realised that we were accessing the ble layer from the ble work queue :(

src/platform/Darwin/BleConnectionDelegateImpl.mm

github-actions · 2023-03-08T14:36:46Z

PR #25522: Size comparison from 0d62a7e to 79ef679

Increases (3 builds for bl702, cc32xx)

platform	target	config	section	`0d62a7e`	`79ef679`	change
bl702	lighting-app	bl702	.debug_info	40860379	40860879	500
			.debug_loc	3423730	`3423751`	21
			.debug_str	3584830	3584892	62
			.strtab	576175	576232	57
		bl702+rpc	.debug_info	45487168	45487738	570
			.debug_loc	3621745	3621766	21
			.debug_str	3989026	3989088	62
			.strtab	637500	637557	57
cc32xx	lock	CC3235SF_LAUNCHXL	.debug_info	20267387	20267468	81
			.debug_loc	2802807	`2802853`	46
			.debug_str	3024017	3024079	62
			.strtab	378514	378571	57

Decreases (2 builds for bl702, cc32xx)

platform	target	config	section	`0d62a7e`	`79ef679`	change	% change
bl702	lighting-app	bl702+rpc	(read/write)	1282311	1282295	-16	-0.0
			.text	1033810	`1033808`	-2	-0.0
cc32xx	lock	CC3235SF_LAUNCHXL	.debug_line	`2659771`	`2659766`	-5	-0.0

Full report (9 builds for bl602, bl702, cc32xx, linux, qpg)

platform	target	config	section	`0d62a7e`	`79ef679`	change	% change
bl602	lighting-app	bl602	(read/write)	1351854	1351854	0	0.0
			.bss	94690	94690	0	0.0
			.data	9744	9744	0	0.0
			.text	1027908	1027908	0	0.0
		bl602+rpc	(read/write)	`1397302`	`1397302`	0	0.0
			.bss	102738	102738	0	0.0
			.data	10136	10136	0	0.0
			.text	1058844	1058844	0	0.0
bl702	lighting-app	bl702		0	0	0	0.0
			(read only)	3358	3358	0	0.0
			(read/write)	1191643	1191643	0	0.0
			.bleromro	6342	6342	0	0.0
			.bleromrw	124	124	0	0.0
			.boot2	292	292	0	0.0
			.bss	69793	69793	0	0.0
			.bss_psram	30160	30160	0	0.0
			.comment	48	48	0	0.0
			.data	4072	4072	0	0.0
			.debug_abbrev	1556782	1556782	0	0.0
			.debug_aranges	134576	134576	0	0.0
			.debug_frame	493312	493312	0	0.0
			.debug_info	40860379	40860879	500	0.0
			.debug_line	5294204	5294204	0	0.0
			.debug_loc	3423730	`3423751`	21	0.0
			.debug_ranges	373192	373192	0	0.0
			.debug_str	3584830	3584892	62	0.0
			.hbn	536	536	0	0.0
			.hbn_noinit	260	260	0	0.0
			.init	342	342	0	0.0
			.init_array	144	144	0	0.0
			.psram	0	0	0	0.0
			.riscv.attributes	47	47	0	0.0
			.rodata	108048	108048	0	0.0
			.rsvd	2960	2960	0	0.0
			.sha_ocram	72	72	0	0.0
			.shstrtab	304	304	0	0.0
			.stack	2048	2048	0	0.0
			.strtab	576175	576232	57	0.0
			.symtab	174128	174128	0	0.0
			.tcm_data	36	36	0	0.0
			.tcmcode	3358	3358	0	0.0
			.text	958466	958466	0	0.0
		bl702+rpc		0	0	0	0.0
			(read only)	3358	3358	0	0.0
			(read/write)	1282311	1282295	-16	-0.0
			.bleromro	6342	6342	0	0.0
			.bleromrw	124	124	0	0.0
			.boot2	292	292	0	0.0
			.bss	77841	77841	0	0.0
			.bss_psram	30432	30432	0	0.0
			.comment	48	48	0	0.0
			.data	4616	4616	0	0.0
			.debug_abbrev	`1708897`	`1708897`	0	0.0
			.debug_aranges	142904	142904	0	0.0
			.debug_frame	521256	521256	0	0.0
			.debug_info	45487168	45487738	570	0.0
			.debug_line	5699547	5699547	0	0.0
			.debug_loc	3621745	3621766	21	0.0
			.debug_ranges	397192	397192	0	0.0
			.debug_str	3989026	3989088	62	0.0
			.hbn	536	536	0	0.0
			.hbn_noinit	260	260	0	0.0
			.init	342	342	0	0.0
			.init_array	160	160	0	0.0
			.psram	0	0	0	0.0
			.riscv.attributes	47	47	0	0.0
			.rodata	122256	122256	0	0.0
			.rsvd	2960	2960	0	0.0
			.sha_ocram	72	72	0	0.0
			.shstrtab	304	304	0	0.0
			.stack	2048	2048	0	0.0
			.strtab	637500	637557	57	0.0
			.symtab	192624	192624	0	0.0
			.tcm_data	36	36	0	0.0
			.tcmcode	3358	3358	0	0.0
			.text	1033810	`1033808`	-2	-0.0
cc32xx	lock	CC3235SF_LAUNCHXL		0	0	0	0.0
			(read only)	644425	644425	0	0.0
			(read/write)	203688	203688	0	0.0
			.ARM.attributes	44	44	0	0.0
			.ARM.exidx	8	8	0	0.0
			.bss	197088	197088	0	0.0
			.comment	194	194	0	0.0
			.data	1480	1480	0	0.0
			.debug_abbrev	930235	930235	0	0.0
			.debug_aranges	87344	87344	0	0.0
			.debug_frame	300044	300044	0	0.0
			.debug_info	20267387	20267468	81	0.0
			.debug_line	`2659771`	`2659766`	-5	-0.0
			.debug_loc	2802807	`2802853`	46	0.0
			.debug_ranges	282960	282960	0	0.0
			.debug_str	3024017	3024079	62	0.0
			.ramVecs	780	780	0	0.0
			.resetVecs	64	64	0	0.0
			.rodata	105929	105929	0	0.0
			.shstrtab	232	232	0	0.0
			.stab	204	204	0	0.0
			.stabstr	441	441	0	0.0
			.stack	2048	2048	0	0.0
			.strtab	378514	378571	57	0.0
			.symtab	256624	256624	0	0.0
			.text	536372	536372	0	0.0
linux	chip-tool-ipv6only	arm64	(read only)	12163892	12163892	0	0.0
			(read/write)	733448	733448	0	0.0
			.bss	34296	34296	0	0.0
			.data	3008	3008	0	0.0
			.data.rel.ro	675552	675552	0	0.0
			.dynamic	560	560	0	0.0
			.got	15384	15384	0	0.0
			.init	24	24	0	0.0
			.init_array	216	216	0	0.0
			.rodata	588436	588436	0	0.0
			.text	9834820	9834820	0	0.0
	thermostat-no-ble	arm64	(read only)	`2521876`	`2521876`	0	0.0
			(read/write)	145256	145256	0	0.0
			.bss	56344	56344	0	0.0
			.data	1784	1784	0	0.0
			.data.rel.ro	77712	77712	0	0.0
			.dynamic	560	560	0	0.0
			.got	5368	5368	0	0.0
			.init	24	24	0	0.0
			.init_array	432	432	0	0.0
			.rodata	150864	150864	0	0.0
			.text	2108576	2108576	0	0.0
qpg	lighting-app	qpg6105+debug	(read/write)	`1153116`	`1153116`	0	0.0
			.bss	99916	99916	0	0.0
			.data	852	852	0	0.0
			.text	600212	600212	0	0.0
	lock-app	qpg6105+debug	(read/write)	1119524	1119524	0	0.0
			.bss	96308	96308	0	0.0
			.data	864	864	0	0.0
			.text	566624	566624	0	0.0

github-actions · 2023-03-09T15:37:40Z

PR #25522: Size comparison from 82da16f to 85d1a42

Decreases (1 build for cc32xx)

platform	target	config	section	`82da16f`	`85d1a42`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL	.debug_info	20267469	20267468	-1	-0.0

Full report (1 build for cc32xx)

platform	target	config	section	`82da16f`	`85d1a42`	change	% change
cc32xx	lock	CC3235SF_LAUNCHXL		0	0	0	0.0
			(read only)	644425	644425	0	0.0
			(read/write)	203688	203688	0	0.0
			.ARM.attributes	44	44	0	0.0
			.ARM.exidx	8	8	0	0.0
			.bss	197088	197088	0	0.0
			.comment	194	194	0	0.0
			.data	1480	1480	0	0.0
			.debug_abbrev	930235	930235	0	0.0
			.debug_aranges	87344	87344	0	0.0
			.debug_frame	300044	300044	0	0.0
			.debug_info	20267469	20267468	-1	-0.0
			.debug_line	2659770	2659770	0	0.0
			.debug_loc	`2802853`	`2802853`	0	0.0
			.debug_ranges	282960	282960	0	0.0
			.debug_str	3024079	3024079	0	0.0
			.ramVecs	780	780	0	0.0
			.resetVecs	64	64	0	0.0
			.rodata	105929	105929	0	0.0
			.shstrtab	232	232	0	0.0
			.stab	204	204	0	0.0
			.stabstr	441	441	0	0.0
			.stack	2048	2048	0	0.0
			.strtab	378571	378571	0	0.0
			.symtab	256624	256624	0	0.0
			.text	536372	536372	0	0.0

vivien-apple self-assigned this Mar 7, 2023

github-actions bot added darwin platform labels Mar 7, 2023

pullapprove bot requested review from woody-apple, xylophone21 and yunhanw-google March 7, 2023 12:43

pullapprove bot added review - pending platform darwin and removed platform darwin labels Mar 7, 2023

bzbarsky-apple reviewed Mar 7, 2023

View reviewed changes

src/platform/Darwin/BleConnectionDelegateImpl.mm Outdated Show resolved Hide resolved

vivien-apple force-pushed the DarwinBle_DataRaceStop branch from 289f772 to cc9d3e3 Compare March 7, 2023 17:45

vivien-apple requested a review from bzbarsky-apple March 7, 2023 17:47

bzbarsky-apple approved these changes Mar 7, 2023

View reviewed changes

vivien-apple force-pushed the DarwinBle_DataRaceStop branch from cc9d3e3 to 79ef679 Compare March 8, 2023 14:04

pullapprove bot requested review from joonhaengHeo and younghak-hwang March 8, 2023 14:05

bzbarsky-apple reviewed Mar 8, 2023

View reviewed changes

src/platform/Darwin/BleConnectionDelegateImpl.mm Outdated Show resolved Hide resolved

[darwin] Race when BleConnection stop is called

85d1a42

vivien-apple force-pushed the DarwinBle_DataRaceStop branch from 79ef679 to 85d1a42 Compare March 9, 2023 15:24

pullapprove bot added review - approved and removed review - pending labels Mar 9, 2023

yufengwangca approved these changes Mar 9, 2023

View reviewed changes

bzbarsky-apple approved these changes Mar 9, 2023

View reviewed changes

vivien-apple merged commit bdee2ef into project-chip:master Mar 10, 2023

lecndav pushed a commit to lecndav/connectedhomeip that referenced this pull request Mar 22, 2023

[darwin] Race when BleConnection stop is called (project-chip#25522)

95d7d0d

mwswartwout pushed a commit to mwswartwout/connectedhomeip that referenced this pull request Mar 27, 2023

[darwin] Race when BleConnection stop is called (project-chip#25522)

a180ea7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[darwin] Race when BleConnection stop is called #25522

[darwin] Race when BleConnection stop is called #25522

vivien-apple commented Mar 7, 2023

github-actions bot commented Mar 7, 2023

bzbarsky-apple left a comment

github-actions bot commented Mar 7, 2023

vivien-apple commented Mar 8, 2023

github-actions bot commented Mar 8, 2023

github-actions bot commented Mar 9, 2023

[darwin] Race when BleConnection stop is called #25522

[darwin] Race when BleConnection stop is called #25522

Conversation

vivien-apple commented Mar 7, 2023

Problem

github-actions bot commented Mar 7, 2023

bzbarsky-apple left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 7, 2023

vivien-apple commented Mar 8, 2023

github-actions bot commented Mar 8, 2023

github-actions bot commented Mar 9, 2023