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

Use default application dispatch if an application hasn't provided one #6053

Merged
merged 2 commits into from
Apr 15, 2021

Conversation

pan-apple
Copy link
Contributor

Problem

Please see issue #6032

Summary of Changes

The exchange context now requires the application to select a message dispatch object that's used for sending the messages. This is done to differentiate between session establishment messages (which are unencrypted) and the application messages (which are encrypted).

Some of the test apps are not registering the delegate. This change adds a default dispatch (for encrypted traffic).

Fixes #6032

@yufengwangca
Copy link
Contributor

@pan-apple

From the crash log, it seems the app crashed at this check
VerifyOrDie(mExchangeMgr != nullptr && GetReferenceCount() > 0);

Send echo request message to Node: 12344321
CHIP:IN: Secure message was encrypted: Msg ID 0
CHIP:IN: Sending msg from 0x000000000001b669 to 0x0000000000bc5c01 at utc time: 701641649 msec
CHIP:IN: Sending secure msg on generic transport
CHIP:IN: Secure msg send status No Error
CHIP:EM: Rxd Ack; Removing MsgId:00000000 from Retrans Table
CHIP:EM: Removed CHIP MsgId:00000000 from RetransTable
CHIP:EM: Sending Standalone Ack for MsgId:00000003
CHIP:EM: Failed to send Solitary ack for MsgId:00000003:4003
Echo Response: 1/1(100.00%) len=15 time=0.000ms
CHIP:EM: Sending Standalone Ack for MsgId:00000003
CHIP:EM: ExchangeContext::SendMessageImpl:Reference count is 0
Aborted (core dumped)

The reference count of ExchangeContext is 0 as the CRMP try to send the ack to EchoResponse message.

The protocol should not call Release directly if CRMP is enabled, instead it should call Close() to give up the ownership of current exchange and the ExchangeMgr will free the exchange after it finish the pending ack.

@pan-apple
Copy link
Contributor Author

@pan-apple

From the crash log, it seems the app crashed at this check
VerifyOrDie(mExchangeMgr != nullptr && GetReferenceCount() > 0);

Send echo request message to Node: 12344321
CHIP:IN: Secure message was encrypted: Msg ID 0
CHIP:IN: Sending msg from 0x000000000001b669 to 0x0000000000bc5c01 at utc time: 701641649 msec
CHIP:IN: Sending secure msg on generic transport
CHIP:IN: Secure msg send status No Error
CHIP:EM: Rxd Ack; Removing MsgId:00000000 from Retrans Table
CHIP:EM: Removed CHIP MsgId:00000000 from RetransTable
CHIP:EM: Sending Standalone Ack for MsgId:00000003
CHIP:EM: Failed to send Solitary ack for MsgId:00000003:4003
Echo Response: 1/1(100.00%) len=15 time=0.000ms
CHIP:EM: Sending Standalone Ack for MsgId:00000003
CHIP:EM: ExchangeContext::SendMessageImpl:Reference count is 0
Aborted (core dumped)

The reference count of ExchangeContext is 0 as the CRMP try to send the ack to EchoResponse message.

The protocol should not call Release directly if CRMP is enabled, instead it should call Close() to give up the ownership of current exchange and the ExchangeMgr will free the exchange after it finish the pending ack.

The application code was not really changed in this PR. I think the crash was a side effect of not being able to send the messages.

@yufengwangca
Copy link
Contributor

@pan-apple
From the crash log, it seems the app crashed at this check
VerifyOrDie(mExchangeMgr != nullptr && GetReferenceCount() > 0);
Send echo request message to Node: 12344321
CHIP:IN: Secure message was encrypted: Msg ID 0
CHIP:IN: Sending msg from 0x000000000001b669 to 0x0000000000bc5c01 at utc time: 701641649 msec
CHIP:IN: Sending secure msg on generic transport
CHIP:IN: Secure msg send status No Error
CHIP:EM: Rxd Ack; Removing MsgId:00000000 from Retrans Table
CHIP:EM: Removed CHIP MsgId:00000000 from RetransTable
CHIP:EM: Sending Standalone Ack for MsgId:00000003
CHIP:EM: Failed to send Solitary ack for MsgId:00000003:4003
Echo Response: 1/1(100.00%) len=15 time=0.000ms
CHIP:EM: Sending Standalone Ack for MsgId:00000003
CHIP:EM: ExchangeContext::SendMessageImpl:Reference count is 0
Aborted (core dumped)
The reference count of ExchangeContext is 0 as the CRMP try to send the ack to EchoResponse message.
The protocol should not call Release directly if CRMP is enabled, instead it should call Close() to give up the ownership of current exchange and the ExchangeMgr will free the exchange after it finish the pending ack.

The application code was not really changed in this PR. I think the crash was a side effect of not being able to send the messages.

Yes, Sending message without an ExchangeDispatch definitely is a problem

@yufengwangca yufengwangca added the hotfix urgent fix needed, can bypass review label Apr 15, 2021
@yufengwangca yufengwangca merged commit f348eb4 into project-chip:master Apr 15, 2021
@pan-apple pan-apple deleted the fix-im-test-crash branch April 15, 2021 17:44
@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from 3510f14

File Section File VM
chip-lighting.elf text 64 64
chip-lighting.elf rodata 48 48
chip-lock.elf text 64 64
chip-lock.elf rodata 48 48
chip-shell.elf text 64 64
chip-shell.elf rodata 48 48
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

sections,vmsize,filesize
.debug_loc,0,960
.debug_line,0,264
.debug_ranges,0,72
text,64,64
rodata,48,48
.debug_abbrev,0,40
.symtab,0,16
.strtab,0,4
.debug_str,0,-4
.debug_aranges,0,-8
.debug_frame,0,-28
.debug_info,0,-2088

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

sections,vmsize,filesize
.debug_loc,0,972
.debug_line,0,260
.debug_ranges,0,72
text,64,64
rodata,48,48
.debug_abbrev,0,40
.symtab,0,16
.strtab,0,4
.debug_str,0,-4
.debug_aranges,0,-8
.debug_frame,0,-28
.debug_info,0,-2088

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

sections,vmsize,filesize
.debug_loc,0,976
.debug_line,0,331
.debug_ranges,0,104
text,64,64
.debug_abbrev,0,56
rodata,48,48
.symtab,0,16
.strtab,0,4
.debug_str,0,-4
.debug_info,0,-1967


@github-actions
Copy link

Size increase report for "esp32-example-build" from 3510f14

File Section File VM
chip-all-clusters-app.elf .flash.text 68 68
chip-all-clusters-app.elf .flash.rodata 48 48
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

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

BLOAT EXECUTION FAILED WITH CODE 1:
bloaty: integer overflow

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

sections,vmsize,filesize
.debug_loc,0,1047
.debug_line,0,561
.debug_ranges,0,136
.flash.text,68,68
.flash.rodata,48,48
.debug_abbrev,0,12
.strtab,0,4
.xt.prop._ZTVN4chip9Transport3BLEE,0,-1
.debug_str,0,-4
.debug_aranges,0,-8
.debug_frame,0,-24
[Unmapped],0,-48
.debug_info,0,-2039


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotfix urgent fix needed, can bypass review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

crash in exchange context when sending multiple messages
3 participants