Skip to content

Commit

Permalink
Mark io target static variables as const to avoid race conditions
Browse files Browse the repository at this point in the history
Also cleans up remaining instances of uninitialized variables in that
target.
  • Loading branch information
fhanau committed Aug 20, 2024
1 parent 770c95b commit bc35e3f
Show file tree
Hide file tree
Showing 36 changed files with 139 additions and 136 deletions.
6 changes: 3 additions & 3 deletions src/workerd/api/crypto/ec.c++
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ kj::Own<EVP_PKEY> ellipticJwkReader(int curveId, SubtleCrypto::JsonWebKey&& keyD
KJ_IF_SOME(alg, keyDataJwk.alg) {
// If this JWK specifies an algorithm, make sure it jives with the hash we were passed via
// importKey().
static std::map<kj::StringPtr, int> ecdsaAlgorithms {
static const std::map<kj::StringPtr, int> ecdsaAlgorithms {
{"ES256", NID_X9_62_prime256v1},
{"ES384", NID_secp384r1},
{"ES512", NID_secp521r1},
Expand Down Expand Up @@ -999,8 +999,8 @@ template <size_t keySize, void (*KeypairInit)(uint8_t[keySize], uint8_t[keySize*
CryptoKeyPair generateKeyImpl(kj::StringPtr normalizedName, int nid,
CryptoKeyUsageSet privateKeyUsages, CryptoKeyUsageSet publicKeyUsages,
bool extractablePrivateKey, kj::StringPtr curveName) {
uint8_t rawPublicKey[keySize];
uint8_t rawPrivateKey[keySize * 2];
uint8_t rawPublicKey[keySize] = {0};
uint8_t rawPrivateKey[keySize * 2] = {0};
KeypairInit(rawPublicKey, rawPrivateKey);

// The private key technically also contains the public key. Why does the keypair function bother
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/crypto/impl.c++
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ kj::String errorsToString(
}
}

SslArrayDisposer SslArrayDisposer::INSTANCE;
const SslArrayDisposer SslArrayDisposer::INSTANCE;

void SslArrayDisposer::disposeImpl(void* firstElement, size_t elementSize, size_t elementCount,
size_t capacity, void (*destroyElement)(void*)) const {
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/crypto/impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ struct CryptoAlgorithm {

class SslArrayDisposer : public kj::ArrayDisposer {
public:
static SslArrayDisposer INSTANCE;
static const SslArrayDisposer INSTANCE;

void disposeImpl(void* firstElement, size_t elementSize, size_t elementCount,
size_t capacity, void (*destroyElement)(void*)) const;
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/crypto/keys.c++
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
namespace workerd::api {

namespace {
static char EMPTY_PASSPHRASE[] = "";
static const char EMPTY_PASSPHRASE[] = "";
} // namespace

kj::StringPtr toStringPtr(KeyType type) {
Expand Down Expand Up @@ -90,7 +90,7 @@ kj::Array<kj::byte> AsymmetricKeyCryptoKeyImpl::exportKeyExt(
auto bio = OSSL_BIO_MEM();

struct EncDetail {
char* pass = &EMPTY_PASSPHRASE[0];
char* pass = const_cast<char*>(&EMPTY_PASSPHRASE[0]);
size_t pass_len = 0;
const EVP_CIPHER* cipher = nullptr;
};
Expand Down
8 changes: 4 additions & 4 deletions src/workerd/api/crypto/rsa.c++
Original file line number Diff line number Diff line change
Expand Up @@ -902,19 +902,19 @@ kj::Own<CryptoKey::Impl> CryptoKey::Impl::importRsa(
KJ_IF_SOME(alg, keyDataJwk.alg) {
// If this JWK specifies an algorithm, make sure it jives with the hash we were passed via
// importKey().
static std::map<kj::StringPtr, const EVP_MD*> rsaShaAlgorithms{
static const std::map<kj::StringPtr, const EVP_MD*> rsaShaAlgorithms{
{"RS1", EVP_sha1()},
{"RS256", EVP_sha256()},
{"RS384", EVP_sha384()},
{"RS512", EVP_sha512()},
};
static std::map<kj::StringPtr, const EVP_MD*> rsaPssAlgorithms{
static const std::map<kj::StringPtr, const EVP_MD*> rsaPssAlgorithms{
{"PS1", EVP_sha1()},
{"PS256", EVP_sha256()},
{"PS384", EVP_sha384()},
{"PS512", EVP_sha512()},
};
static std::map<kj::StringPtr, const EVP_MD*> rsaOaepAlgorithms{
static const std::map<kj::StringPtr, const EVP_MD*> rsaOaepAlgorithms{
{"RSA-OAEP", EVP_sha1()},
{"RSA-OAEP-256", EVP_sha256()},
{"RSA-OAEP-384", EVP_sha384()},
Expand Down Expand Up @@ -996,7 +996,7 @@ kj::Own<CryptoKey::Impl> CryptoKey::Impl::importRsaRaw(
KJ_IF_SOME(alg, keyDataJwk.alg) {
// If this JWK specifies an algorithm, make sure it jives with the hash we were passed via
// importKey().
static std::map<kj::StringPtr, const EVP_MD*> rsaAlgorithms{
static const std::map<kj::StringPtr, const EVP_MD*> rsaAlgorithms{
{"RS1", EVP_sha1()},
{"RS256", EVP_sha256()},
{"RS384", EVP_sha384()},
Expand Down
8 changes: 4 additions & 4 deletions src/workerd/api/crypto/x509.c++
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ bool printGeneralName(BIO* out, const GENERAL_NAME* gen) {
} else if (gen->type == GEN_RID) {
// Unlike OpenSSL's default implementation, never print the OID as text and
// instead always print its numeric representation.
char oline[256];
char oline[256] = {0};
OBJ_obj2txt(oline, sizeof(oline), gen->d.rid, true);
BIO_printf(out, "Registered ID:%s", oline);
} else if (gen->type == GEN_OTHERNAME) {
Expand Down Expand Up @@ -314,7 +314,7 @@ bool safeX509InfoAccessPrint(BIO* out, X509_EXTENSION* ext) {
if (i != 0)
BIO_write(out, "\n", 1);

char objtmp[80];
char objtmp[80] = {0};
i2t_ASN1_OBJECT(objtmp, sizeof(objtmp), desc->method);
BIO_printf(out, "%s - ", objtmp);
if (!(ok = printGeneralName(out, desc->location))) {
Expand Down Expand Up @@ -491,7 +491,7 @@ kj::Maybe<jsg::JsObject> getX509NameObject(jsg::Lock& js, X509* cert) {
// If OpenSSL knows the type, use the short name of the type as the key, and
// the numeric representation of the type's OID otherwise.
int type_nid = OBJ_obj2nid(type);
char type_buf[80];
char type_buf[80] = {0};
const char* type_str;
if (type_nid != NID_undef) {
type_str = OBJ_nid2sn(type_nid);
Expand Down Expand Up @@ -540,7 +540,7 @@ struct StackOfXASN1Disposer: public kj::Disposer {
sk_ASN1_OBJECT_pop_free(ptr, ASN1_OBJECT_free);
}
};
StackOfXASN1Disposer stackOfXASN1Disposer;
constexpr StackOfXASN1Disposer stackOfXASN1Disposer;
} // namespace

kj::Maybe<jsg::Ref<X509Certificate>> X509Certificate::parse(kj::Array<const kj::byte> raw) {
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/encoding.c++
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,9 @@ Encoding getEncodingForLabel(kj::StringPtr label) {
}
} // namespace

kj::Array<const kj::byte> TextDecoder::EMPTY =
const kj::Array<const kj::byte> TextDecoder::EMPTY =
kj::Array<const kj::byte>(&DUMMY, 0, kj::NullArrayDisposer::instance);
TextDecoder::DecodeOptions TextDecoder::DEFAULT_OPTIONS = TextDecoder::DecodeOptions();
const TextDecoder::DecodeOptions TextDecoder::DEFAULT_OPTIONS = TextDecoder::DecodeOptions();

kj::Maybe<IcuDecoder> IcuDecoder::create(Encoding encoding, bool fatal, bool ignoreBom) {
UErrorCode status = U_ZERO_ERROR;
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/encoding.h
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,9 @@ class TextDecoder final: public jsg::Object {
DecoderImpl decoder;
ConstructorOptions ctorOptions;

static DecodeOptions DEFAULT_OPTIONS;
static const DecodeOptions DEFAULT_OPTIONS;
static constexpr kj::byte DUMMY = 0;
static kj::Array<const kj::byte> EMPTY;
static const kj::Array<const kj::byte> EMPTY;
};

// Implements the TextEncoder interface as prescribed by:
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/eventsource.c++
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ void EventSource::start(jsg::Lock& js) {

auto fetcher = i.options.fetcher.map([](jsg::Ref<Fetcher>& f) { return f.addRef(); });

static auto handleError = [](auto& js, auto& self, kj::String message) {
static constexpr auto handleError = [](auto& js, auto& self, kj::String message) {
auto ex = js.domException(kj::str("AbortError"), kj::mv(message));
auto handle = KJ_ASSERT_NONNULL(ex.tryGetHandle(js));
self->notifyError(js, jsg::JsValue(handle));
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/form-data.c++
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ struct FormDataHeaderTable {
};

const FormDataHeaderTable& getFormDataHeaderTable() {
static FormDataHeaderTable table({});
static const FormDataHeaderTable table({});
return table;
}

Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/node/crypto-keys.c++
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ CryptoKey::AsymmetricKeyDetails CryptoImpl::getAsymmetricKeyDetail(
}

kj::StringPtr CryptoImpl::getAsymmetricKeyType(jsg::Lock& js, jsg::Ref<CryptoKey> key) {
static std::map<kj::StringPtr, kj::StringPtr> mapping{
static const std::map<kj::StringPtr, kj::StringPtr> mapping{
{"RSASSA-PKCS1-v1_5", "rsa"},
{"RSA-PSS", "rsa"},
{"RSA-OAEP", "rsa"},
Expand Down
8 changes: 4 additions & 4 deletions src/workerd/api/sql.c++
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,19 @@ double SqlStorage::getDatabaseSize() {
return pages * getPageSize();
}

bool SqlStorage::isAllowedName(kj::StringPtr name) {
bool SqlStorage::isAllowedName(kj::StringPtr name) const {
return !name.startsWith("_cf_");
}

bool SqlStorage::isAllowedTrigger(kj::StringPtr name) {
bool SqlStorage::isAllowedTrigger(kj::StringPtr name) const {
return true;
}

void SqlStorage::onError(kj::StringPtr message) {
void SqlStorage::onError(kj::StringPtr message) const {
JSG_ASSERT(false, Error, message);
}

bool SqlStorage::allowTransactions() {
bool SqlStorage::allowTransactions() const {
if (IoContext::hasCurrent()) {
IoContext::current().logWarningOnce(
"To execute a transaction, please use the state.storage.transaction() API instead of the "
Expand Down
8 changes: 4 additions & 4 deletions src/workerd/api/sql.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ class SqlStorage final: public jsg::Object, private SqliteDatabase::Regulator {
visitor.visit(storage);
}

bool isAllowedName(kj::StringPtr name) override;
bool isAllowedTrigger(kj::StringPtr name) override;
void onError(kj::StringPtr message) override;
bool allowTransactions() override;
bool isAllowedName(kj::StringPtr name) const override;
bool isAllowedTrigger(kj::StringPtr name) const override;
void onError(kj::StringPtr message) const override;
bool allowTransactions() const override;

IoPtr<SqliteDatabase> sqlite;
jsg::Ref<DurableObjectStorage> storage;
Expand Down
5 changes: 3 additions & 2 deletions src/workerd/api/streams/queue-test.c++
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ using ReadContinuation = jsg::Promise<ReadResult>(ReadResult&&);
using CloseContinuation = jsg::Promise<void>(ReadResult&&);
using ReadErrorContinuation = jsg::Promise<ReadResult>(jsg::Value&&);

kj::UnwindDetector unwindDetector;
const kj::MutexGuarded<kj::UnwindDetector> unwindDetectorMutex;

template <typename Signature>
struct MustCall;
Expand Down Expand Up @@ -61,7 +61,8 @@ struct MustCall<Ret(Args...)> {
location(location) {}

~MustCall() {
if (!unwindDetector.isUnwinding()) {
auto unwindDetector = unwindDetectorMutex.lockExclusive();
if (!unwindDetector->isUnwinding()) {
KJ_ASSERT(called == expected,
kj::str("MustCall function was not called ", expected,
" times. [actual: ", called , "]"), location);
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/api/streams/readable.c++
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ ReadableStream::Reader ReadableStream::getReader(
jsg::Ref<ReadableStream::ReadableStreamAsyncIterator> ReadableStream::values(
jsg::Lock& js,
jsg::Optional<ValuesOptions> options) {
static auto defaultOptions = ValuesOptions {};
static const auto defaultOptions = ValuesOptions {};
return jsg::alloc<ReadableStreamAsyncIterator>(AsyncIteratorState {
.ioContext = ioContext,
.reader = ReadableStreamDefaultReader::constructor(js, JSG_THIS),
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/api/url.c++
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ namespace {

bool isSpecialScheme(kj::StringPtr scheme) {
// TODO(cleanup): Move this to kj::Url.
static std::set<kj::StringPtr> specialSchemes{
static const std::set<kj::StringPtr> specialSchemes{
"ftp", "file", "gopher", "http", "https", "ws", "wss"};
return specialSchemes.count(scheme);
}

kj::Maybe<kj::StringPtr> defaultPortForScheme(kj::StringPtr scheme) {
static std::map<kj::StringPtr, kj::StringPtr> defaultPorts {
static const std::map<kj::StringPtr, kj::StringPtr> defaultPorts {
{ "ftp", "21" },
{ "gopher", "70" },
{ "http", "80" },
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/io/actor-cache.c++
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace workerd {
// hit this limit, so this is just a sanity check.
static constexpr size_t MAX_ACTOR_STORAGE_RPC_WORDS = (16u << 20) / sizeof(capnp::word);

ActorCache::Hooks ActorCache::Hooks::DEFAULT;
const ActorCache::Hooks ActorCache::Hooks::DEFAULT;

namespace {

Expand Down
4 changes: 2 additions & 2 deletions src/workerd/io/actor-cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -258,15 +258,15 @@ class ActorCache final: public ActorCacheInterface {
virtual void storageReadCompleted(kj::Duration latency) {}
virtual void storageWriteCompleted(kj::Duration latency) {}

static Hooks DEFAULT;
static const Hooks DEFAULT;
};

static constexpr auto SHUTDOWN_ERROR_MESSAGE =
"broken.ignored; jsg.Error: "
"Durable Object storage is no longer accessible."_kj;

ActorCache(rpc::ActorStorage::Stage::Client storage, const SharedLru& lru, OutputGate& gate,
Hooks& hooks = Hooks::DEFAULT);
Hooks& hooks = const_cast<Hooks&>(Hooks::DEFAULT));
~ActorCache() noexcept(false);

kj::Maybe<SqliteDatabase&> getSqliteDatabase() override { return kj::none; }
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/io/actor-sqlite.c++
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ kj::Maybe<kj::Promise<void>> ActorSqlite::onNoPendingFlush() {
return outputGate.wait();
}

ActorSqlite::Hooks ActorSqlite::Hooks::DEFAULT = ActorSqlite::Hooks{};
const ActorSqlite::Hooks ActorSqlite::Hooks::DEFAULT = ActorSqlite::Hooks{};

kj::Maybe<kj::Own<void>> ActorSqlite::Hooks::armAlarmHandler(kj::Date scheduledTime, bool noCache) {
JSG_FAIL_REQUIRE(Error, "alarms are not yet implemented for SQLite-backed Durable Objects");
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/io/actor-sqlite.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class ActorSqlite final: public ActorCacheInterface, private kj::TaskSet::ErrorH
virtual kj::Maybe<kj::Own<void>> armAlarmHandler(kj::Date scheduledTime, bool noCache);
virtual void cancelDeferredAlarmDeletion();

static Hooks DEFAULT;
static const Hooks DEFAULT;
};

// Constructs ActorSqlite, arranging to honor the output gate, that is, any writes to the
Expand All @@ -41,7 +41,7 @@ class ActorSqlite final: public ActorCacheInterface, private kj::TaskSet::ErrorH
// machines before being considered durable.
explicit ActorSqlite(kj::Own<SqliteDatabase> dbParam, OutputGate& outputGate,
kj::Function<kj::Promise<void>()> commitCallback,
Hooks& hooks = Hooks::DEFAULT);
Hooks& hooks = const_cast<Hooks&>(Hooks::DEFAULT));

bool isCommitScheduled() { return !currentTxn.is<NoTxn>(); }

Expand Down
4 changes: 2 additions & 2 deletions src/workerd/io/compatibility-date.c++
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ kj::Array<const ParsedField> makeFieldTable(
} // namespace

kj::Array<kj::StringPtr> decompileCompatibilityFlagsForFl(CompatibilityFlags::Reader input) {
static auto fieldTable = makeFieldTable(
static const auto fieldTable = makeFieldTable(
capnp::Schema::from<CompatibilityFlags>().getFields());

kj::Vector<kj::StringPtr> enableFlags;
Expand Down Expand Up @@ -331,7 +331,7 @@ kj::Maybe<PythonSnapshotRelease::Reader> getPythonSnapshotRelease(
uint latestFieldOrdinal = 0;
kj::Maybe<PythonSnapshotRelease::Reader> result;

static auto fieldTable =
static const auto fieldTable =
makePythonSnapshotFieldTable(capnp::Schema::from<CompatibilityFlags>().getFields());

for (auto field: fieldTable) {
Expand Down
4 changes: 2 additions & 2 deletions src/workerd/io/io-gate.c++
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

namespace workerd {

InputGate::Hooks InputGate::Hooks::DEFAULT;
const InputGate::Hooks InputGate::Hooks::DEFAULT;

InputGate::InputGate(Hooks& hooks): InputGate(hooks, kj::newPromiseAndFulfiller<void>()) {}
InputGate::InputGate(Hooks& hooks, kj::PromiseFulfillerPair<void> paf)
Expand Down Expand Up @@ -306,7 +306,7 @@ OutputGate::OutputGate(Hooks& hooks)
: hooks(hooks), pastLocksPromise(kj::Promise<void>(kj::READY_NOW).fork()) {}
OutputGate::~OutputGate() noexcept(false) {}

OutputGate::Hooks OutputGate::Hooks::DEFAULT;
const OutputGate::Hooks OutputGate::Hooks::DEFAULT;

kj::Own<kj::PromiseFulfiller<void>> OutputGate::lock() {
auto paf = kj::newPromiseAndFulfiller<void>();
Expand Down
8 changes: 4 additions & 4 deletions src/workerd/io/io-gate.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ class InputGate {
virtual void inputGateWaiterAdded() {}
virtual void inputGateWaiterRemoved() {}

static Hooks DEFAULT;
static const Hooks DEFAULT;
};

InputGate(Hooks& hooks = Hooks::DEFAULT);
InputGate(Hooks& hooks = const_cast<Hooks&>(Hooks::DEFAULT));
~InputGate() noexcept;

class CriticalSection;
Expand Down Expand Up @@ -235,10 +235,10 @@ class OutputGate {
virtual void outputGateWaiterAdded() {}
virtual void outputGateWaiterRemoved() {}

static Hooks DEFAULT;
static const Hooks DEFAULT;
};

OutputGate(Hooks& hooks = Hooks::DEFAULT);
OutputGate(Hooks& hooks = const_cast<Hooks&>(Hooks::DEFAULT));
~OutputGate() noexcept(false);

// Block all future `wait()` calls until `promise` completes. Returns a wrapper around `promise`.
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/io/worker.c++
Original file line number Diff line number Diff line change
Expand Up @@ -1744,7 +1744,7 @@ void Worker::handleLog(jsg::Lock& js, ConsoleMode consoleMode, LogLevel level,
KJ_LOG(INFO, "console.log()", message());
} else {
// Write to stdio if allowed by console mode
static ColorMode COLOR_MODE = permitsColor();
static const ColorMode COLOR_MODE = permitsColor();
#if _WIN32
static bool STDOUT_TTY = _isatty(_fileno(stdout));
static bool STDERR_TTY = _isatty(_fileno(stderr));
Expand Down
2 changes: 1 addition & 1 deletion src/workerd/jsg/dom-exception.c++
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ kj::StringPtr DOMException::getMessage() {
}

int DOMException::getCode() {
static std::map<kj::StringPtr, int> legacyCodes{
static const std::map<kj::StringPtr, int> legacyCodes{
#define MAP_ENTRY(name, code, friendlyName) {friendlyName, code},
JSG_DOM_EXCEPTION_FOR_EACH_ERROR_NAME(MAP_ENTRY)
#undef MAP_ENTRY
Expand Down
Loading

0 comments on commit bc35e3f

Please sign in to comment.