Page MenuHomePhabricator
Differential D9774

[Flow202][web][skip-ci] [8/x] Fix Flow issues with types of variables that get mutated
ClosedPublic
Actions

Authored by ashoat on Nov 8 2023, 11:36 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Mar 18, 2:03 AM
Unknown Object (File)
Tue, Mar 18, 2:03 AM
Unknown Object (File)
Tue, Mar 18, 2:03 AM
Unknown Object (File)
Tue, Mar 18, 2:03 AM
Unknown Object (File)
Tue, Mar 18, 2:03 AM
Unknown Object (File)
Tue, Mar 18, 2:02 AM
Unknown Object (File)
Tue, Mar 18, 2:02 AM
Unknown Object (File)
Tue, Mar 18, 1:52 AM
View All Files
Subscribers
tomek
will

Details

Reviewers
atul
Commits
rCOMM703a41589dda: [Flow202][web][skip-ci] [8/x] Fix Flow issues with types of variables that get…
Summary

This the web counterpart of both D9706 and D9692 (I combined them for web).

There are two main categories of changes here, and both pertain to variables getting mutated:

  1. let declarations where the variable is reassigned later
  2. const declarations of mutable collections that are mutated later

In both of these cases, Flow now wants us to annotate the declaration to cover both the type of the data that's initialized, and also the mutations that later occur.

NOTE: CI will fail on this diff. I considered the possibility of fixing Flow errors BEFORE upgrading Flow, but it wasn't possible... in some cases, the fixes to support the new version of Flow caused errors in the old version. I could have hidden these type errors with $FlowFixMe lines and then later revert those, but that seemed like too much busy work.

Depends on D9773

Test Plan

Confirm the Flow errors go away

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat created this revision.Nov 8 2023, 11:36 AM
Herald added subscribers: will, tomek. · View Herald TranscriptNov 8 2023, 11:36 AM
ashoat added a child revision: D9775: [Flow202][web][skip-ci] [9/x] Fix Flow issues with types being underconstrained.Nov 8 2023, 11:37 AM
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 8 2023, 11:50 AM
Harbormaster failed remote builds in B23922: Diff 32960!
ashoat requested review of this revision.Nov 8 2023, 11:51 AM
atul accepted this revision.Nov 9 2023, 11:44 AM
This revision is now accepted and ready to land.Nov 9 2023, 11:44 AM
ashoat updated this revision to Diff 33160.Nov 13 2023, 2:33 PM

Rebase

ashoat updated this revision to Diff 33162.Nov 13 2023, 2:39 PM

Rebase and fix a new error

Harbormaster failed remote builds in B24062: Diff 33160!Nov 13 2023, 2:43 PM
Harbormaster failed remote builds in B24064: Diff 33162!Nov 13 2023, 2:50 PM
ashoat mentioned this in D9858: [Flow202][keyserver][skip-ci] [9/x] Fix Flow issues with types of variables that get mutated.Nov 13 2023, 3:01 PM
ashoat retitled this revision from [Flow202][web] [8/x] Fix Flow issues with types of variables that get mutated to [Flow202][web][skip-ci] [8/x] Fix Flow issues with types of variables that get mutated.Nov 19 2023, 5:02 PM
ashoat updated this revision to Diff 33401.Nov 19 2023, 5:30 PM

Rebase

Harbormaster completed remote builds in B24263: Diff 33401.Nov 19 2023, 5:39 PM
ashoat mentioned this in D10010: [Flow202][native][skip-ci] [26/x] Fix Flow issues with types of variables that get mutated.Nov 27 2023, 12:45 AM
ashoat updated this revision to Diff 33773.Nov 27 2023, 2:06 PM

Rebase

Harbormaster completed remote builds in B24541: Diff 33773.Nov 27 2023, 2:06 PM
Closed by commit rCOMM703a41589dda: [Flow202][web][skip-ci] [8/x] Fix Flow issues with types of variables that get… (authored by ashoat). · Explain WhyNov 27 2023, 3:26 PM
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMM703a41589dda: [Flow202][web][skip-ci] [8/x] Fix Flow issues with types of variables that get….
ashoat mentioned this in rCOMMbe075d8192e3: [Flow202][keyserver][skip-ci] [9/x] Fix Flow issues with types of variables….
ashoat mentioned this in rCOMM77d36444612a: [Flow202][native][skip-ci] [26/x] Fix Flow issues with types of variables that….Nov 27 2023, 3:28 PM

Revision Contents
Changeset List

Diff 33401

View Options

web/app.react.js

#include "CreateNewBackupReactor.h" #include "CreateNewBackupReactor.h"
#include "DatabaseManager.h" #include "DatabaseManager.h"
#include "GlobalTools.h" #include "GlobalTools.h"
#include "Tools.h" #include "Tools.h"
#include "blob_client/src/lib.rs.h"
namespace comm { namespace comm {
namespace network { namespace network {
namespace reactor { namespace reactor {
std::string CreateNewBackupReactor::generateBackupID() { std::string CreateNewBackupReactor::generateBackupID() {
if (this->deviceID.empty()) { if (this->deviceID.empty()) {
throw std::runtime_error( throw std::runtime_error(
"trying to generate a backup ID with an empty device ID"); "trying to generate a backup ID with an empty device ID");
▲ Show 20 Lines • Show All 44 Lines • ▼ Show 20 Lines case State::DATA_HASH: {
if (database::DatabaseManager::getInstance().findBackupItem( if (database::DatabaseManager::getInstance().findBackupItem(
this->userID, this->backupID) != nullptr) { this->userID, this->backupID) != nullptr) {
throw std::runtime_error( throw std::runtime_error(
"Backup with id [" + this->backupID + "] for user [" + "Backup with id [" + this->backupID + "] for user [" +
this->userID + "] already exists, creation aborted"); this->userID + "] already exists, creation aborted");
} }
response->set_backupid(this->backupID); response->set_backupid(this->backupID);
this->holder = tools::generateHolder(this->dataHash, this->backupID); this->holder = tools::generateHolder(this->dataHash, this->backupID);
// todo:blob perform put:initialize bool dataExists = false;
tomekUnsubmitted
Not Done
Inline Actions

This isn't used anywhere

tomek: This isn't used anywhere
karolAuthorUnsubmitted
Done
Inline Actions

right

karol: right
try {
put_client_initialize_cxx();
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kHolder),
this->holder.c_str());
put_client_blocking_read_cxx(); // todo this should be avoided
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
tomekUnsubmitted
Not Done
Inline Actions

We should create some tasks that cover things mentioned in the comment

tomek: We should create some tasks that cover things mentioned in the comment
karolAuthorUnsubmitted
Done
Inline Actions

https://linear.app/comm/issue/ENG-1724/solve-the-problem-with-blocked-reactors-in-grpc

karol: https://linear.app/comm/issue/ENG-1724/solve-the-problem-with-blocked-reactors-in-grpc
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kBlobHash),
this->dataHash.c_str());
rust::String responseStr =
put_client_blocking_read_cxx(); // todo this should be avoided
tomekUnsubmitted
Not Done
Inline Actions

This is confusing. We call put_client_blocking_read_cxx twice, and we ignore the result once. Could you explain (preferably in code comment) what's going on here?

tomek: This is confusing. We call `put_client_blocking_read_cxx` twice, and we ignore the result once.
karolAuthorUnsubmitted
Done
Inline Actions

We read the response to unblock the reading queue. grpc in the bidi stream always yields a response even if it's empty. And it is in this case, that's why we ignore it, but we read so the queue's not blocked (this queue is a rust's channel with MPSC_CHANNEL_BUFFER_CAPACITY limit).

karol: We read the response to unblock the reading queue. grpc in the bidi stream always yields a…
// (blocking); we should be able to
// ignore responses; we probably
// want to delegate performing ops
// to separate threads in the base
// reactors
// data exists?
if ((bool)tools::charPtrToInt(responseStr.c_str())) {
return std::make_unique<ServerBidiReactorStatus>(
grpc::Status::OK, true);
}
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
return nullptr; return nullptr;
} }
case State::DATA_CHUNKS: { case State::DATA_CHUNKS: {
// todo:blob perform put:add chunk if (request.mutable_newcompactionchunk()->empty()) {
// (std::move(*request.mutable_newcompactionchunk()) return std::make_unique<ServerBidiReactorStatus>(grpc::Status::OK);
}
try {
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kDataChunk),
std::string(std::move(*request.mutable_newcompactionchunk()))
.c_str());
put_client_blocking_read_cxx(); // todo this should be avoided
tomekUnsubmitted
Not Done
Inline Actions

Why do we need to read after write?

tomek: Why do we need to read after write?
karolAuthorUnsubmitted
Done
Inline Actions

Explained in the other comment

karol: Explained in the other comment
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
return nullptr; return nullptr;
} }
} }
throw std::runtime_error("new backup - invalid state"); throw std::runtime_error("new backup - invalid state");
} }
void CreateNewBackupReactor::terminateCallback() { void CreateNewBackupReactor::terminateCallback() {
const std::lock_guard<std::mutex> lock(this->reactorStateMutex); const std::lock_guard<std::mutex> lock(this->reactorStateMutex);
// todo:blob perform put:add chunk ("") try {
// todo:blob perform put:wait for completion put_client_terminate_cxx();
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
// TODO add recovery data // TODO add recovery data
// TODO handle attachments holders // TODO handle attachments holders
database::BackupItem backupItem( database::BackupItem backupItem(
this->userID, this->userID,
this->backupID, this->backupID,
tools::getCurrentTimestamp(), tools::getCurrentTimestamp(),
tools::generateRandomString(), tools::generateRandomString(),
this->holder, this->holder,
{}); {});
database::DatabaseManager::getInstance().putBackupItem(backupItem); database::DatabaseManager::getInstance().putBackupItem(backupItem);
} }
} // namespace reactor } // namespace reactor
} // namespace network } // namespace network
} // namespace comm } // namespace comm
View Options

web/calendar/filter-panel.react.js

#include "CreateNewBackupReactor.h" #include "CreateNewBackupReactor.h"
#include "DatabaseManager.h" #include "DatabaseManager.h"
#include "GlobalTools.h" #include "GlobalTools.h"
#include "Tools.h" #include "Tools.h"
#include "blob_client/src/lib.rs.h"
namespace comm { namespace comm {
namespace network { namespace network {
namespace reactor { namespace reactor {
std::string CreateNewBackupReactor::generateBackupID() { std::string CreateNewBackupReactor::generateBackupID() {
if (this->deviceID.empty()) { if (this->deviceID.empty()) {
throw std::runtime_error( throw std::runtime_error(
"trying to generate a backup ID with an empty device ID"); "trying to generate a backup ID with an empty device ID");
▲ Show 20 Lines • Show All 44 Lines • ▼ Show 20 Lines case State::DATA_HASH: {
if (database::DatabaseManager::getInstance().findBackupItem( if (database::DatabaseManager::getInstance().findBackupItem(
this->userID, this->backupID) != nullptr) { this->userID, this->backupID) != nullptr) {
throw std::runtime_error( throw std::runtime_error(
"Backup with id [" + this->backupID + "] for user [" + "Backup with id [" + this->backupID + "] for user [" +
this->userID + "] already exists, creation aborted"); this->userID + "] already exists, creation aborted");
} }
response->set_backupid(this->backupID); response->set_backupid(this->backupID);
this->holder = tools::generateHolder(this->dataHash, this->backupID); this->holder = tools::generateHolder(this->dataHash, this->backupID);
// todo:blob perform put:initialize bool dataExists = false;
tomekUnsubmitted
Not Done
Inline Actions

This isn't used anywhere

tomek: This isn't used anywhere
karolAuthorUnsubmitted
Done
Inline Actions

right

karol: right
try {
put_client_initialize_cxx();
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kHolder),
this->holder.c_str());
put_client_blocking_read_cxx(); // todo this should be avoided
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
tomekUnsubmitted
Not Done
Inline Actions

We should create some tasks that cover things mentioned in the comment

tomek: We should create some tasks that cover things mentioned in the comment
karolAuthorUnsubmitted
Done
Inline Actions

https://linear.app/comm/issue/ENG-1724/solve-the-problem-with-blocked-reactors-in-grpc

karol: https://linear.app/comm/issue/ENG-1724/solve-the-problem-with-blocked-reactors-in-grpc
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kBlobHash),
this->dataHash.c_str());
rust::String responseStr =
put_client_blocking_read_cxx(); // todo this should be avoided
tomekUnsubmitted
Not Done
Inline Actions

This is confusing. We call put_client_blocking_read_cxx twice, and we ignore the result once. Could you explain (preferably in code comment) what's going on here?

tomek: This is confusing. We call `put_client_blocking_read_cxx` twice, and we ignore the result once.
karolAuthorUnsubmitted
Done
Inline Actions

We read the response to unblock the reading queue. grpc in the bidi stream always yields a response even if it's empty. And it is in this case, that's why we ignore it, but we read so the queue's not blocked (this queue is a rust's channel with MPSC_CHANNEL_BUFFER_CAPACITY limit).

karol: We read the response to unblock the reading queue. grpc in the bidi stream always yields a…
// (blocking); we should be able to
// ignore responses; we probably
// want to delegate performing ops
// to separate threads in the base
// reactors
// data exists?
if ((bool)tools::charPtrToInt(responseStr.c_str())) {
return std::make_unique<ServerBidiReactorStatus>(
grpc::Status::OK, true);
}
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
return nullptr; return nullptr;
} }
case State::DATA_CHUNKS: { case State::DATA_CHUNKS: {
// todo:blob perform put:add chunk if (request.mutable_newcompactionchunk()->empty()) {
// (std::move(*request.mutable_newcompactionchunk()) return std::make_unique<ServerBidiReactorStatus>(grpc::Status::OK);
}
try {
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kDataChunk),
std::string(std::move(*request.mutable_newcompactionchunk()))
.c_str());
put_client_blocking_read_cxx(); // todo this should be avoided
tomekUnsubmitted
Not Done
Inline Actions

Why do we need to read after write?

tomek: Why do we need to read after write?
karolAuthorUnsubmitted
Done
Inline Actions

Explained in the other comment

karol: Explained in the other comment
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
return nullptr; return nullptr;
} }
} }
throw std::runtime_error("new backup - invalid state"); throw std::runtime_error("new backup - invalid state");
} }
void CreateNewBackupReactor::terminateCallback() { void CreateNewBackupReactor::terminateCallback() {
const std::lock_guard<std::mutex> lock(this->reactorStateMutex); const std::lock_guard<std::mutex> lock(this->reactorStateMutex);
// todo:blob perform put:add chunk ("") try {
// todo:blob perform put:wait for completion put_client_terminate_cxx();
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
// TODO add recovery data // TODO add recovery data
// TODO handle attachments holders // TODO handle attachments holders
database::BackupItem backupItem( database::BackupItem backupItem(
this->userID, this->userID,
this->backupID, this->backupID,
tools::getCurrentTimestamp(), tools::getCurrentTimestamp(),
tools::generateRandomString(), tools::generateRandomString(),
this->holder, this->holder,
{}); {});
database::DatabaseManager::getInstance().putBackupItem(backupItem); database::DatabaseManager::getInstance().putBackupItem(backupItem);
} }
} // namespace reactor } // namespace reactor
} // namespace network } // namespace network
} // namespace comm } // namespace comm
View Options

web/chat/chat-input-bar.react.js

#include "CreateNewBackupReactor.h" #include "CreateNewBackupReactor.h"
#include "DatabaseManager.h" #include "DatabaseManager.h"
#include "GlobalTools.h" #include "GlobalTools.h"
#include "Tools.h" #include "Tools.h"
#include "blob_client/src/lib.rs.h"
namespace comm { namespace comm {
namespace network { namespace network {
namespace reactor { namespace reactor {
std::string CreateNewBackupReactor::generateBackupID() { std::string CreateNewBackupReactor::generateBackupID() {
if (this->deviceID.empty()) { if (this->deviceID.empty()) {
throw std::runtime_error( throw std::runtime_error(
"trying to generate a backup ID with an empty device ID"); "trying to generate a backup ID with an empty device ID");
▲ Show 20 Lines • Show All 44 Lines • ▼ Show 20 Lines case State::DATA_HASH: {
if (database::DatabaseManager::getInstance().findBackupItem( if (database::DatabaseManager::getInstance().findBackupItem(
this->userID, this->backupID) != nullptr) { this->userID, this->backupID) != nullptr) {
throw std::runtime_error( throw std::runtime_error(
"Backup with id [" + this->backupID + "] for user [" + "Backup with id [" + this->backupID + "] for user [" +
this->userID + "] already exists, creation aborted"); this->userID + "] already exists, creation aborted");
} }
response->set_backupid(this->backupID); response->set_backupid(this->backupID);
this->holder = tools::generateHolder(this->dataHash, this->backupID); this->holder = tools::generateHolder(this->dataHash, this->backupID);
// todo:blob perform put:initialize bool dataExists = false;
tomekUnsubmitted
Not Done
Inline Actions

This isn't used anywhere

tomek: This isn't used anywhere
karolAuthorUnsubmitted
Done
Inline Actions

right

karol: right
try {
put_client_initialize_cxx();
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kHolder),
this->holder.c_str());
put_client_blocking_read_cxx(); // todo this should be avoided
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
tomekUnsubmitted
Not Done
Inline Actions

We should create some tasks that cover things mentioned in the comment

tomek: We should create some tasks that cover things mentioned in the comment
karolAuthorUnsubmitted
Done
Inline Actions

https://linear.app/comm/issue/ENG-1724/solve-the-problem-with-blocked-reactors-in-grpc

karol: https://linear.app/comm/issue/ENG-1724/solve-the-problem-with-blocked-reactors-in-grpc
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kBlobHash),
this->dataHash.c_str());
rust::String responseStr =
put_client_blocking_read_cxx(); // todo this should be avoided
tomekUnsubmitted
Not Done
Inline Actions

This is confusing. We call put_client_blocking_read_cxx twice, and we ignore the result once. Could you explain (preferably in code comment) what's going on here?

tomek: This is confusing. We call `put_client_blocking_read_cxx` twice, and we ignore the result once.
karolAuthorUnsubmitted
Done
Inline Actions

We read the response to unblock the reading queue. grpc in the bidi stream always yields a response even if it's empty. And it is in this case, that's why we ignore it, but we read so the queue's not blocked (this queue is a rust's channel with MPSC_CHANNEL_BUFFER_CAPACITY limit).

karol: We read the response to unblock the reading queue. grpc in the bidi stream always yields a…
// (blocking); we should be able to
// ignore responses; we probably
// want to delegate performing ops
// to separate threads in the base
// reactors
// data exists?
if ((bool)tools::charPtrToInt(responseStr.c_str())) {
return std::make_unique<ServerBidiReactorStatus>(
grpc::Status::OK, true);
}
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
return nullptr; return nullptr;
} }
case State::DATA_CHUNKS: { case State::DATA_CHUNKS: {
// todo:blob perform put:add chunk if (request.mutable_newcompactionchunk()->empty()) {
// (std::move(*request.mutable_newcompactionchunk()) return std::make_unique<ServerBidiReactorStatus>(grpc::Status::OK);
}
try {
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kDataChunk),
std::string(std::move(*request.mutable_newcompactionchunk()))
.c_str());
put_client_blocking_read_cxx(); // todo this should be avoided
tomekUnsubmitted
Not Done
Inline Actions

Why do we need to read after write?

tomek: Why do we need to read after write?
karolAuthorUnsubmitted
Done
Inline Actions

Explained in the other comment

karol: Explained in the other comment
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
return nullptr; return nullptr;
} }
} }
throw std::runtime_error("new backup - invalid state"); throw std::runtime_error("new backup - invalid state");
} }
void CreateNewBackupReactor::terminateCallback() { void CreateNewBackupReactor::terminateCallback() {
const std::lock_guard<std::mutex> lock(this->reactorStateMutex); const std::lock_guard<std::mutex> lock(this->reactorStateMutex);
// todo:blob perform put:add chunk ("") try {
// todo:blob perform put:wait for completion put_client_terminate_cxx();
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
// TODO add recovery data // TODO add recovery data
// TODO handle attachments holders // TODO handle attachments holders
database::BackupItem backupItem( database::BackupItem backupItem(
this->userID, this->userID,
this->backupID, this->backupID,
tools::getCurrentTimestamp(), tools::getCurrentTimestamp(),
tools::generateRandomString(), tools::generateRandomString(),
this->holder, this->holder,
{}); {});
database::DatabaseManager::getInstance().putBackupItem(backupItem); database::DatabaseManager::getInstance().putBackupItem(backupItem);
} }
} // namespace reactor } // namespace reactor
} // namespace network } // namespace network
} // namespace comm } // namespace comm
View Options

web/chat/text-message.react.js

#include "CreateNewBackupReactor.h" #include "CreateNewBackupReactor.h"
#include "DatabaseManager.h" #include "DatabaseManager.h"
#include "GlobalTools.h" #include "GlobalTools.h"
#include "Tools.h" #include "Tools.h"
#include "blob_client/src/lib.rs.h"
namespace comm { namespace comm {
namespace network { namespace network {
namespace reactor { namespace reactor {
std::string CreateNewBackupReactor::generateBackupID() { std::string CreateNewBackupReactor::generateBackupID() {
if (this->deviceID.empty()) { if (this->deviceID.empty()) {
throw std::runtime_error( throw std::runtime_error(
"trying to generate a backup ID with an empty device ID"); "trying to generate a backup ID with an empty device ID");
▲ Show 20 Lines • Show All 44 Lines • ▼ Show 20 Lines case State::DATA_HASH: {
if (database::DatabaseManager::getInstance().findBackupItem( if (database::DatabaseManager::getInstance().findBackupItem(
this->userID, this->backupID) != nullptr) { this->userID, this->backupID) != nullptr) {
throw std::runtime_error( throw std::runtime_error(
"Backup with id [" + this->backupID + "] for user [" + "Backup with id [" + this->backupID + "] for user [" +
this->userID + "] already exists, creation aborted"); this->userID + "] already exists, creation aborted");
} }
response->set_backupid(this->backupID); response->set_backupid(this->backupID);
this->holder = tools::generateHolder(this->dataHash, this->backupID); this->holder = tools::generateHolder(this->dataHash, this->backupID);
// todo:blob perform put:initialize bool dataExists = false;
tomekUnsubmitted
Not Done
Inline Actions

This isn't used anywhere

tomek: This isn't used anywhere
karolAuthorUnsubmitted
Done
Inline Actions

right

karol: right
try {
put_client_initialize_cxx();
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kHolder),
this->holder.c_str());
put_client_blocking_read_cxx(); // todo this should be avoided
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
tomekUnsubmitted
Not Done
Inline Actions

We should create some tasks that cover things mentioned in the comment

tomek: We should create some tasks that cover things mentioned in the comment
karolAuthorUnsubmitted
Done
Inline Actions

https://linear.app/comm/issue/ENG-1724/solve-the-problem-with-blocked-reactors-in-grpc

karol: https://linear.app/comm/issue/ENG-1724/solve-the-problem-with-blocked-reactors-in-grpc
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kBlobHash),
this->dataHash.c_str());
rust::String responseStr =
put_client_blocking_read_cxx(); // todo this should be avoided
tomekUnsubmitted
Not Done
Inline Actions

This is confusing. We call put_client_blocking_read_cxx twice, and we ignore the result once. Could you explain (preferably in code comment) what's going on here?

tomek: This is confusing. We call `put_client_blocking_read_cxx` twice, and we ignore the result once.
karolAuthorUnsubmitted
Done
Inline Actions

We read the response to unblock the reading queue. grpc in the bidi stream always yields a response even if it's empty. And it is in this case, that's why we ignore it, but we read so the queue's not blocked (this queue is a rust's channel with MPSC_CHANNEL_BUFFER_CAPACITY limit).

karol: We read the response to unblock the reading queue. grpc in the bidi stream always yields a…
// (blocking); we should be able to
// ignore responses; we probably
// want to delegate performing ops
// to separate threads in the base
// reactors
// data exists?
if ((bool)tools::charPtrToInt(responseStr.c_str())) {
return std::make_unique<ServerBidiReactorStatus>(
grpc::Status::OK, true);
}
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
return nullptr; return nullptr;
} }
case State::DATA_CHUNKS: { case State::DATA_CHUNKS: {
// todo:blob perform put:add chunk if (request.mutable_newcompactionchunk()->empty()) {
// (std::move(*request.mutable_newcompactionchunk()) return std::make_unique<ServerBidiReactorStatus>(grpc::Status::OK);
}
try {
put_client_write_cxx(
tools::getBlobPutField(blob::PutRequest::DataCase::kDataChunk),
std::string(std::move(*request.mutable_newcompactionchunk()))
.c_str());
put_client_blocking_read_cxx(); // todo this should be avoided
tomekUnsubmitted
Not Done
Inline Actions

Why do we need to read after write?

tomek: Why do we need to read after write?
karolAuthorUnsubmitted
Done
Inline Actions

Explained in the other comment

karol: Explained in the other comment
// (blocking); we should be able to
// ignore responses; we probably want to
// delegate performing ops to separate
// threads in the base reactors
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
return nullptr; return nullptr;
} }
} }
throw std::runtime_error("new backup - invalid state"); throw std::runtime_error("new backup - invalid state");
} }
void CreateNewBackupReactor::terminateCallback() { void CreateNewBackupReactor::terminateCallback() {
const std::lock_guard<std::mutex> lock(this->reactorStateMutex); const std::lock_guard<std::mutex> lock(this->reactorStateMutex);
// todo:blob perform put:add chunk ("") try {
// todo:blob perform put:wait for completion put_client_terminate_cxx();
} catch (std::exception &e) {
throw std::runtime_error(
e.what()); // todo in base reactors we can just handle std exception
// instead of keep rethrowing here
}
// TODO add recovery data // TODO add recovery data
// TODO handle attachments holders // TODO handle attachments holders
database::BackupItem backupItem( database::BackupItem backupItem(
this->userID, this->userID,
this->backupID, this->backupID,
tools::getCurrentTimestamp(), tools::getCurrentTimestamp(),
tools::generateRandomString(), tools::generateRandomString(),
this->holder, this->holder,
{}); {});
database::DatabaseManager::getInstance().putBackupItem(backupItem); database::DatabaseManager::getInstance().putBackupItem(backupItem);
} }
} // namespace reactor } // namespace reactor
} // namespace network } // namespace network
} // namespace comm } // namespace comm
View Options

web/components/community-actions-menu.react.js

Loading...
View Options

web/components/menu-item.react.js

Loading...
View Options

web/input/input-state-container.react.js

Loading...
View Options

web/loading-indicator.react.js

Loading...
View Options

web/media/blob-utils.js

Loading...
View Options

web/media/image-utils.js

Loading...
View Options

web/media/media-utils.js

Loading...
View Options

web/modals/chat/message-results-modal.react.js

Loading...
View Options

web/modals/threads/settings/thread-settings-modal.react.js

Loading...
View Options

web/olm/olm-utils.js

Loading...
View Options

web/redux/action-types.js

Loading...
View Options

web/redux/persist.js

Loading...
View Options

web/settings/relationship/add-users-list.react.js

Loading...
View Options

web/settings/tunnelbroker-message-list.react.js

Loading...
View Options

web/url-utils.js

Loading...