Skip to content

Commit 796f169

Browse files
nivi-applebzbarsky-apple
authored andcommitted
Handle the output event of type TransferSession::OutputEventType::kIn… (project-chip#37324)
* Handle the output event of type TransferSession::OutputEventType::kInternalError in the ProcessOuputEvents processing loop - The transfer session state machine generates an output event of type TransferSession::OutputEventType::kInternalError in certain error scenarios which put the transfer session in a bad state and when that happens, we need to unwind the processing loop for events and clean up. * Update src/protocols/bdx/AsyncTransferFacilitator.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> * Address review comments * Update src/protocols/bdx/AsyncTransferFacilitator.cpp Co-authored-by: Boris Zbarsky <bzbarsky@apple.com> --------- Co-authored-by: Boris Zbarsky <bzbarsky@apple.com>
1 parent 92f71c4 commit 796f169

File tree

3 files changed

+27
-171
lines changed

3 files changed

+27
-171
lines changed

.github/workflows/examples-linux-standalone.yaml

+2-162
Original file line numberDiff line numberDiff line change
@@ -55,170 +55,9 @@ jobs:
5555
with:
5656
gh-context: ${{ toJson(github) }}
5757

58-
- name: Build Standalone cert tool
59-
run: |
60-
./scripts/run_in_build_env.sh \
61-
"./scripts/build/build_examples.py \
62-
--target linux-x64-chip-cert \
63-
build"
64-
- name: Build minmdns example with platform dns
65-
run: |
66-
./scripts/run_in_build_env.sh \
67-
"./scripts/build/build_examples.py \
68-
--target linux-x64-address-resolve-tool-platform-mdns-ipv6only \
69-
build"
70-
- name: Build example Standalone chip tool
71-
run: |
72-
./scripts/run_in_build_env.sh \
73-
"./scripts/build/build_examples.py \
74-
--target linux-x64-chip-tool \
75-
build"
76-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
77-
linux debug chip-tool \
78-
out/linux-x64-chip-tool/chip-tool \
79-
/tmp/bloat_reports/
80-
- name: Build example Standalone Shell
81-
run: |
82-
./scripts/run_in_build_env.sh \
83-
"./scripts/build/build_examples.py \
84-
--target linux-x64-shell \
85-
build"
86-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
87-
linux debug shell \
88-
out/linux-x64-shell/chip-shell \
89-
/tmp/bloat_reports/
90-
- name: Build example Standalone All Clusters Server
91-
run: |
92-
./scripts/run_in_build_env.sh \
93-
"./scripts/build/build_examples.py \
94-
--target linux-x64-all-clusters \
95-
build"
96-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
97-
linux debug all-clusters-app \
98-
out/linux-x64-all-clusters/chip-all-clusters-app \
99-
/tmp/bloat_reports/
100-
- name: Clean out build output
101-
run: rm -rf ./out
102-
- name: Build example Standalone All Clusters Minimal Server
103-
run: |
104-
./scripts/run_in_build_env.sh \
105-
"./scripts/build/build_examples.py \
106-
--target linux-x64-all-clusters-minimal \
107-
build"
108-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
109-
linux debug all-clusters-minimal-app \
110-
out/linux-x64-all-clusters-minimal/chip-all-clusters-minimal-app \
111-
/tmp/bloat_reports/
112-
- name: Build example TV app
113-
run: |
114-
./scripts/run_in_build_env.sh \
115-
"./scripts/build/build_examples.py \
116-
--target linux-x64-tv-app \
117-
build"
118-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
119-
linux debug tv-app \
120-
out/linux-x64-tv-app/chip-tv-app \
121-
/tmp/bloat_reports/
122-
- name: Build example Standalone TV Casting App
123-
run: |
124-
./scripts/run_in_build_env.sh \
125-
"./scripts/build/build_examples.py \
126-
--target linux-x64-tv-casting-app \
127-
build"
128-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
129-
linux debug tv-casting-app \
130-
out/linux-x64-tv-casting-app/chip-tv-casting-app \
131-
/tmp/bloat_reports/
132-
- name: Build example lighting app with RPCs and UI
133-
run: |
134-
./scripts/run_in_build_env.sh \
135-
"./scripts/build/build_examples.py \
136-
--target linux-x64-light-rpc-with-ui \
137-
build"
138-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
139-
linux debug+rpc+ui lighting-app \
140-
out/linux-x64-light-rpc-with-ui/chip-lighting-app \
141-
/tmp/bloat_reports/
142-
- name: Clean out build output
143-
run: rm -rf ./out
144-
- name: Build example Standalone Bridge
145-
run: |
146-
./scripts/run_in_build_env.sh \
147-
"./scripts/build/build_examples.py \
148-
--target linux-x64-bridge \
149-
build"
150-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
151-
linux debug bridge-app \
152-
out/linux-x64-bridge/chip-bridge-app \
153-
/tmp/bloat_reports/
154-
- name: Build example OTA Provider
155-
run: |
156-
./scripts/run_in_build_env.sh \
157-
"./scripts/build/build_examples.py \
158-
--target linux-x64-ota-provider \
159-
build"
160-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
161-
linux debug ota-provider-app \
162-
out/linux-x64-ota-provider/chip-ota-provider-app \
163-
/tmp/bloat_reports/
164-
- name: Build example OTA Requestor
165-
run: |
166-
./scripts/run_in_build_env.sh \
167-
"./scripts/build/build_examples.py \
168-
--target linux-x64-ota-requestor \
169-
build"
170-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
171-
linux debug ota-requestor-app \
172-
out/linux-x64-ota-requestor/chip-ota-requestor-app \
173-
/tmp/bloat_reports/
17458
- name: Clean out build output
17559
run: rm -rf ./out
176-
- name: Build example Standalone Lock App
177-
run: |
178-
./scripts/run_in_build_env.sh \
179-
"./scripts/build/build_examples.py \
180-
--target linux-x64-lock-no-thread \
181-
build"
182-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
183-
linux debug lock-app \
184-
out/linux-x64-lock-no-thread/chip-lock-app \
185-
/tmp/bloat_reports/
186-
- name: Build example contact sensor with UI
187-
run: |
188-
./scripts/run_in_build_env.sh \
189-
"./scripts/build/build_examples.py \
190-
--target linux-x64-contact-sensor-no-ble-with-ui \
191-
build"
192-
- name: Build example Air Purifier
193-
run: |
194-
./scripts/run_in_build_env.sh \
195-
"./scripts/build/build_examples.py \
196-
--target linux-x64-air-purifier \
197-
build"
198-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
199-
linux debug air-purifier-app \
200-
out/linux-x64-air-purifier/chip-air-purifier-app \
201-
/tmp/bloat_reports/
202-
- name: Build example Fabric Admin
203-
run: |
204-
./scripts/run_in_build_env.sh \
205-
"./scripts/build/build_examples.py \
206-
--target linux-x64-fabric-admin-rpc \
207-
build"
208-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
209-
linux debug fabric-admin \
210-
out/linux-x64-fabric-admin-rpc/fabric-admin \
211-
/tmp/bloat_reports/
212-
- name: Build example Fabric Bridge App
213-
run: |
214-
./scripts/run_in_build_env.sh \
215-
"./scripts/build/build_examples.py \
216-
--target linux-x64-fabric-bridge-no-ble-rpc \
217-
build"
218-
.environment/pigweed-venv/bin/python3 scripts/tools/memory/gh_sizes.py \
219-
linux debug fabric-bridge-app \
220-
out/linux-x64-fabric-bridge-no-ble-rpc/fabric-bridge-app \
221-
/tmp/bloat_reports/
60+
22261
- name: Build example Fabric Sync
22362
run: |
22463
./scripts/run_in_build_env.sh \
@@ -229,6 +68,7 @@ jobs:
22968
linux debug fabric-sync \
23069
out/linux-x64-fabric-sync-no-ble/fabric-sync \
23170
/tmp/bloat_reports/
71+
23272
- name: Uploading Size Reports
23373
uses: ./.github/actions/upload-size-reports
23474
if: ${{ !env.ACT }}

src/protocols/bdx/AsyncTransferFacilitator.cpp

+22-8
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,17 @@ void AsyncTransferFacilitator::ProcessOutputEvents()
6363
mTransfer.GetNextAction(outEvent);
6464
while (outEvent.EventType != TransferSession::OutputEventType::kNone)
6565
{
66+
67+
// If the transfer session state machine generates an event of type TransferSession::OutputEventType::kInternalError,
68+
// indicating that the session is in a bad state, it will keep doing that thereafter.
69+
//
70+
// So stop trying to process events, and go ahead and destroy ourselves to clean up the transfer.
71+
if (outEvent.EventType == TransferSession::OutputEventType::kInternalError)
72+
{
73+
mDestroySelfAfterProcessingEvents = true;
74+
break;
75+
}
76+
6677
if (outEvent.EventType == TransferSession::OutputEventType::kMsgToSend)
6778
{
6879
CHIP_ERROR err = SendMessage(outEvent.msgTypeData, outEvent.MsgData);
@@ -170,22 +181,25 @@ void AsyncResponder::NotifyEventHandled(const TransferSession::OutputEventType e
170181
ChipLogDetail(BDX, "NotifyEventHandled : Event %s Error %" CHIP_ERROR_FORMAT,
171182
TransferSession::OutputEvent::TypeToString(eventType), status.Format());
172183

173-
// If this is the end of the transfer (whether a clean end, or some sort of error condition), ensure that
174-
// we destroy ourselves after processing any output events that might have been generated by
175-
// the transfer session to handle end-of-transfer (e.g. in some error conditions it might want to send
176-
// out a status report).
184+
// If this is the end of the transfer (whether a clean end, or some sort of error condition), ensure
185+
// that we destroy ourselves after unwinding the processing loop in the ProcessOutputEvents API.
186+
// We can ignore the status for these output events because none of them are supposed to result in
187+
// us sending a StatusReport, and that's all we use the status for.
188+
//
189+
// In particular, for kTransferTimeout, kAckEOFReceived, and kStatusReceived per spec we
190+
// are not supposed to reply with a StatusReport. And for kInternalError the state machine
191+
// is in an unrecoverable state of some sort, and we should stop trying to make use of it.
177192
if (eventType == TransferSession::OutputEventType::kAckEOFReceived ||
178193
eventType == TransferSession::OutputEventType::kInternalError ||
179194
eventType == TransferSession::OutputEventType::kTransferTimeout ||
180195
eventType == TransferSession::OutputEventType::kStatusReceived)
181196
{
182197
mDestroySelfAfterProcessingEvents = true;
183198
}
184-
185-
// If there was an error handling the output event, this should notify the transfer object to abort transfer so it can send a
186-
// status report across the exchange when we call ProcessOutputEvents below.
187-
if (status != CHIP_NO_ERROR)
199+
else if (status != CHIP_NO_ERROR)
188200
{
201+
// If there was an error handling the output event, this should notify the transfer object to abort transfer
202+
// so it can send a status report across the exchange when we call ProcessOutputEvents below.
189203
mTransfer.AbortTransfer(GetBdxStatusCodeFromChipError(status));
190204
}
191205

src/protocols/bdx/AsyncTransferFacilitator.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ class AsyncTransferFacilitator : public Messaging::ExchangeDelegate
7272
*/
7373
virtual void DestroySelf() = 0;
7474

75-
// Calling ProcessOutputEvents can destroy this object before the call returns.
75+
/**
76+
* Calling ProcessOutputEvents can destroy this object before the call returns.
77+
*/
7678
void ProcessOutputEvents();
7779

7880
// The transfer session corresponding to this AsyncTransferFacilitator object.

0 commit comments

Comments
 (0)