Page MenuHomePhabricator

[services] Backup/Blob - Make sure Finish is only called once
ClosedPublic

Authored by karol on Apr 8 2022, 2:33 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Oct 13, 3:42 PM
Unknown Object (File)
Sun, Oct 13, 3:42 PM
Unknown Object (File)
Sun, Oct 13, 3:42 PM
Unknown Object (File)
Sun, Oct 13, 3:42 PM
Unknown Object (File)
Sun, Oct 13, 3:42 PM
Unknown Object (File)
Sun, Oct 13, 3:39 PM
Unknown Object (File)
Sep 23 2024, 9:06 AM
Unknown Object (File)
Sep 15 2024, 6:55 AM

Details

Summary

Depends on D3636

We should make 100% sure that Finish (the method provided by gRPC) is called no more than once. Otherwise, it's going to crash.

Test Plan
cd services
yarn run-blob-service
yarn run-backup-service

Diff Detail

Repository
rCOMM Comm
Branch
backup-blob
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

services/backup/docker-server/contents/server/src/Reactors/server/base-reactors/ServerBidiReactorBase.h
70 ↗(On Diff #11218)

this->finished

79 ↗(On Diff #11218)

this->finished

services/backup/docker-server/contents/server/src/Reactors/server/base-reactors/ServerReadReactorBase.h
43 ↗(On Diff #11218)

this->finished

47 ↗(On Diff #11218)

this->finished

ashoat requested changes to this revision.Apr 10 2022, 7:01 PM

(I only mentioned four locations but there are more)

This revision now requires changes to proceed.Apr 10 2022, 7:01 PM
ashoat added 1 blocking reviewer(s): tomek.

Don't have context on the rest, but looks good to me

Are we sure that this is properly synchronized and we don't need e.g. atomic?

This revision is now accepted and ready to land.Apr 13 2022, 2:48 AM
In D3664#102317, @palys-swm wrote:

Are we sure that this is properly synchronized and we don't need e.g. atomic?

Good point, we can use it.