From c0908b9f7529f5c3d8d1d727eca87623a6101718 Mon Sep 17 00:00:00 2001 From: Matt Broadstone Date: Thu, 29 Feb 2024 17:46:29 +0000 Subject: [PATCH] SERVER-86818 Rename our custom Map implementation to BSONAwareMap GitOrigin-RevId: bcf8bc34491d05927cd3574c3ee0ee8f7657ca2b --- .eslintrc.yml | 1 + ...atement_transaction_atomicity_isolation.js | 2 +- jstests/core/map1.js | 2 +- jstests/core/query/js/js_global_scope.js | 2 +- jstests/libs/cycle_detection.js | 6 ++--- src/mongo/db/commands/mr_test.cpp | 2 +- src/mongo/db/mongod_main.cpp | 2 +- src/mongo/db/pipeline/accumulator_js_test.cpp | 2 +- .../pipeline/expression_javascript_test.cpp | 2 +- src/mongo/db/pipeline/javascript_execution.h | 3 +-- src/mongo/dbtests/dbtests.cpp | 2 +- src/mongo/s/mongos_main.cpp | 2 +- src/mongo/scripting/engine.cpp | 7 +----- src/mongo/scripting/engine.h | 17 +++++++------ src/mongo/scripting/engine_none.cpp | 2 +- src/mongo/scripting/mozjs/engine.cpp | 11 ++++---- src/mongo/scripting/mozjs/engine.h | 25 +++++++++---------- src/mongo/scripting/mozjs/implscope.cpp | 11 +++++--- .../scripting/mozjs/module_loader_test.cpp | 8 +++--- src/mongo/scripting/mozjs/objectwrapper.cpp | 7 ++++++ src/mongo/scripting/mozjs/objectwrapper.h | 3 +++ src/mongo/shell/db.js | 2 +- src/mongo/shell/mongo_main.cpp | 2 +- src/mongo/shell/types.js | 23 ++++++++--------- 24 files changed, 77 insertions(+), 69 deletions(-) diff --git a/.eslintrc.yml b/.eslintrc.yml index e4d7e3c9473..1a5d49c085d 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -46,6 +46,7 @@ globals: SessionOptions: true CollInfos: true CountDownLatch: true + BSONAwareMap: true # FCV-related latestFCV: true diff --git a/jstests/concurrency/fsm_workloads/multi_statement_transaction_atomicity_isolation.js b/jstests/concurrency/fsm_workloads/multi_statement_transaction_atomicity_isolation.js index 04b9e58e891..841337edc87 100644 --- a/jstests/concurrency/fsm_workloads/multi_statement_transaction_atomicity_isolation.js +++ b/jstests/concurrency/fsm_workloads/multi_statement_transaction_atomicity_isolation.js @@ -78,7 +78,7 @@ export const $config = (function() { } function checkNumUpdatedByEachTransaction(documents) { - const updateCounts = new Map(); + const updateCounts = new BSONAwareMap(); for (let doc of documents) { for (let op of doc.order) { diff --git a/jstests/core/map1.js b/jstests/core/map1.js index ea2dec5db69..b1641e96930 100644 --- a/jstests/core/map1.js +++ b/jstests/core/map1.js @@ -1,6 +1,6 @@ function basic1(key, lookup, shouldFail) { - var m = new Map(); + var m = new BSONAwareMap(); m.put(key, 17); var out = m.get(lookup || key); diff --git a/jstests/core/query/js/js_global_scope.js b/jstests/core/query/js/js_global_scope.js index c2085cde225..9a09c058d74 100644 --- a/jstests/core/query/js/js_global_scope.js +++ b/jstests/core/query/js/js_global_scope.js @@ -201,4 +201,4 @@ assert.eq( "}\n" + "return true;" }) - .itcount()); \ No newline at end of file + .itcount()); diff --git a/jstests/libs/cycle_detection.js b/jstests/libs/cycle_detection.js index a481226b3fc..7802ece4502 100644 --- a/jstests/libs/cycle_detection.js +++ b/jstests/libs/cycle_detection.js @@ -7,7 +7,7 @@ export function Graph() { } const nodes = new Set(); - const adjList = new Map(); + const adjList = new BSONAwareMap(); this.addEdge = function addEdge(fromNode, toNode) { nodes.add(fromNode); @@ -44,7 +44,7 @@ export function Graph() { * - http://www.cs.cornell.edu/courses/cs2112/2012sp/lectures/lec24/lec24-12sp.html */ this.findCycle = function findCycle() { - const state = new Map(); + const state = new BSONAwareMap(); for (let node of nodes) { state.put(node, State.kNotYetVisited); } @@ -84,7 +84,7 @@ export function Graph() { // A cycle has been detected during the recursive call to doDepthFirstSearch(). // Unless we've already closed the loop, the (node, otherNode) edge must be part // of it. Note that we use friendlyEqual() to match the definition of sameness - // as the mongo shell's Map type. + // as the mongo shell's BSONAwareMap type. if (!friendlyEqual(result[0], result[result.length - 1])) { result.unshift(node); } diff --git a/src/mongo/db/commands/mr_test.cpp b/src/mongo/db/commands/mr_test.cpp index 4823456a9be..6ebd6009a9f 100644 --- a/src/mongo/db/commands/mr_test.cpp +++ b/src/mongo/db/commands/mr_test.cpp @@ -447,7 +447,7 @@ const NamespaceString MapReduceCommandTest::outputNss = void MapReduceCommandTest::setUp() { ServiceContextMongoDTest::setUp(); - ScriptEngine::setup(); + ScriptEngine::setup(ExecutionEnvironment::Server); auto service = getServiceContext(); DBDirectClientFactory::get(service).registerImplementation( [](OperationContext* opCtx) { return std::make_unique(opCtx); }); diff --git a/src/mongo/db/mongod_main.cpp b/src/mongo/db/mongod_main.cpp index 2a200349d69..fe59d18b72b 100644 --- a/src/mongo/db/mongod_main.cpp +++ b/src/mongo/db/mongod_main.cpp @@ -731,7 +731,7 @@ ExitCode _initAndListen(ServiceContext* serviceContext, int listenPort) { startMongoDFTDC(serviceContext); if (mongodGlobalParams.scriptingEnabled) { - ScriptEngine::setup(); + ScriptEngine::setup(ExecutionEnvironment::Server); } const auto isStandalone = diff --git a/src/mongo/db/pipeline/accumulator_js_test.cpp b/src/mongo/db/pipeline/accumulator_js_test.cpp index 481c21471e9..7ae483be670 100644 --- a/src/mongo/db/pipeline/accumulator_js_test.cpp +++ b/src/mongo/db/pipeline/accumulator_js_test.cpp @@ -79,7 +79,7 @@ private: void MapReduceFixture::setUp() { ServiceContextMongoDTest::setUp(); - ScriptEngine::setup(false); + ScriptEngine::setup(ExecutionEnvironment::Server); } void MapReduceFixture::tearDown() { diff --git a/src/mongo/db/pipeline/expression_javascript_test.cpp b/src/mongo/db/pipeline/expression_javascript_test.cpp index 51a8be82a20..565bf1a2d56 100644 --- a/src/mongo/db/pipeline/expression_javascript_test.cpp +++ b/src/mongo/db/pipeline/expression_javascript_test.cpp @@ -90,7 +90,7 @@ private: void MapReduceFixture::setUp() { ServiceContextMongoDTest::setUp(); - ScriptEngine::setup(false); + ScriptEngine::setup(ExecutionEnvironment::Server); } void MapReduceFixture::tearDown() { diff --git a/src/mongo/db/pipeline/javascript_execution.h b/src/mongo/db/pipeline/javascript_execution.h index 895544bbd0b..31f97485345 100644 --- a/src/mongo/db/pipeline/javascript_execution.h +++ b/src/mongo/db/pipeline/javascript_execution.h @@ -57,8 +57,7 @@ public: /** * Create or get a pointer to a JsExecution instance, capable of invoking Javascript functions * and reading the return value. If `loadStoredProcedures` is true, this will load all stored - * procedures from database unless 'disableLoadStored' is set on the global ScriptEngine. The - * JsExecution* returned is owned by 'opCtx'. + * procedures from database. The JsExecution* returned is owned by 'opCtx'. */ static JsExecution* get(OperationContext* opCtx, const BSONObj& scope, diff --git a/src/mongo/dbtests/dbtests.cpp b/src/mongo/dbtests/dbtests.cpp index e38e79fe3ed..a9fc28491a8 100644 --- a/src/mongo/dbtests/dbtests.cpp +++ b/src/mongo/dbtests/dbtests.cpp @@ -266,7 +266,7 @@ int dbtestsMain(int argc, char** argv) { service, std::make_unique(storageMock.get())); AuthorizationManager::get(service->getService())->setAuthEnabled(false); - ScriptEngine::setup(); + ScriptEngine::setup(ExecutionEnvironment::Server); return mongo::dbtests::runDbTests(argc, argv); } diff --git a/src/mongo/s/mongos_main.cpp b/src/mongo/s/mongos_main.cpp index d49a6ce1545..9779c9c2793 100644 --- a/src/mongo/s/mongos_main.cpp +++ b/src/mongo/s/mongos_main.cpp @@ -910,7 +910,7 @@ ExitCode runMongosServer(ServiceContext* serviceContext) { TimeElapsedBuilderScopedTimer scopedTimer(serviceContext->getFastClockSource(), "Set up script engine", &startupTimeElapsedBuilder); - ScriptEngine::setup(); + ScriptEngine::setup(ExecutionEnvironment::Server); } { diff --git a/src/mongo/scripting/engine.cpp b/src/mongo/scripting/engine.cpp index 0d7c71de871..775089d4dbe 100644 --- a/src/mongo/scripting/engine.cpp +++ b/src/mongo/scripting/engine.cpp @@ -96,10 +96,7 @@ static std::unique_ptr globalScriptEngine; } // namespace -ScriptEngine::ScriptEngine(bool disableLoadStored) - : _disableLoadStored(disableLoadStored), _scopeInitCallback() {} - -ScriptEngine::~ScriptEngine() {} +ScriptEngine::ScriptEngine() : _scopeInitCallback() {} Scope::Scope() : _localDBName(DatabaseName::kEmpty), @@ -257,8 +254,6 @@ void Scope::validateObjectIdString(const string& str) { } void Scope::loadStored(OperationContext* opCtx, bool ignoreNotConnected) { - if (!getGlobalScriptEngine()->_disableLoadStored) - return; if (_localDBName.isEmpty()) { if (ignoreNotConnected) return; diff --git a/src/mongo/scripting/engine.h b/src/mongo/scripting/engine.h index 48b0c5cfccc..01f04e633b5 100644 --- a/src/mongo/scripting/engine.h +++ b/src/mongo/scripting/engine.h @@ -231,13 +231,15 @@ protected: bool _lastRetIsNativeCode; // v8 only: set to true if eval'd script returns a native func }; +enum class ExecutionEnvironment { Server, TestRunner }; + class ScriptEngine : public KillOpListenerInterface { ScriptEngine(const ScriptEngine&) = delete; ScriptEngine& operator=(const ScriptEngine&) = delete; public: - ScriptEngine(bool disableLoadStored); - virtual ~ScriptEngine(); + ScriptEngine(); + virtual ~ScriptEngine() = default; virtual Scope* newScope() { return createScope(); @@ -268,11 +270,9 @@ public: virtual void setLoadPath(const std::string& loadPath) = 0; /** - * Calls the constructor for the Global ScriptEngine. 'disableLoadStored' causes future calls to - * the function Scope::loadStored(), which would otherwise load stored procedures, to be - * ignored. + * Calls the constructor for the Global ScriptEngine. */ - static void setup(bool disableLoadStored = true); + static void setup(ExecutionEnvironment environment); static void dropScopeCache(); /** gets a scope from the pool or a new one if pool is empty @@ -285,6 +285,10 @@ public: const DatabaseName& db, const std::string& scopeType); + using ScopeCallback = void (*)(Scope&); + ScopeCallback getScopeInitCallback() { + return _scopeInitCallback; + }; void setScopeInitCallback(void (*func)(Scope&)) { _scopeInitCallback = func; } @@ -302,7 +306,6 @@ public: virtual void interruptAll() {} static std::string getInterpreterVersionString(); - const bool _disableLoadStored; protected: virtual Scope* createScope() = 0; diff --git a/src/mongo/scripting/engine_none.cpp b/src/mongo/scripting/engine_none.cpp index 42b02fe4a24..14bd05865d7 100644 --- a/src/mongo/scripting/engine_none.cpp +++ b/src/mongo/scripting/engine_none.cpp @@ -32,7 +32,7 @@ #include "mongo/scripting/engine.h" namespace mongo { -void ScriptEngine::setup(bool disableLoadStored) { +void ScriptEngine::setup(ExecutionEnvironment environment) { // noop } diff --git a/src/mongo/scripting/mozjs/engine.cpp b/src/mongo/scripting/mozjs/engine.cpp index 0109935288b..24f582568a5 100644 --- a/src/mongo/scripting/mozjs/engine.cpp +++ b/src/mongo/scripting/mozjs/engine.cpp @@ -66,11 +66,12 @@ void DisableExtraThreads(); namespace mongo { -void ScriptEngine::setup(bool disableLoadStored) { - if (getGlobalScriptEngine()) +void ScriptEngine::setup(ExecutionEnvironment environment) { + if (getGlobalScriptEngine()) { return; + } - setGlobalScriptEngine(new mozjs::MozJSScriptEngine(disableLoadStored)); + setGlobalScriptEngine(new mozjs::MozJSScriptEngine(environment)); if (hasGlobalServiceContext()) { getGlobalServiceContext()->registerKillOpListener(getGlobalScriptEngine()); @@ -84,8 +85,8 @@ std::string ScriptEngine::getInterpreterVersionString() { namespace mozjs { -MozJSScriptEngine::MozJSScriptEngine(bool disableLoadStored) - : ScriptEngine(disableLoadStored), _loadPath(boost::filesystem::current_path().string()) { +MozJSScriptEngine::MozJSScriptEngine(ExecutionEnvironment environment) + : _executionEnvironment(environment), _loadPath(boost::filesystem::current_path().string()) { uassert(ErrorCodes::JSInterpreterFailure, "Failed to JS_Init()", JS_Init()); js::DisableExtraThreads(); } diff --git a/src/mongo/scripting/mozjs/engine.h b/src/mongo/scripting/mozjs/engine.h index ddc3d90a1d2..d952567b751 100644 --- a/src/mongo/scripting/mozjs/engine.h +++ b/src/mongo/scripting/mozjs/engine.h @@ -37,7 +37,6 @@ #include "mongo/scripting/deadline_monitor.h" #include "mongo/scripting/engine.h" #include "mongo/stdx/unordered_map.h" -#include "mongo/util/concurrency/mutex.h" namespace mongo { namespace mozjs { @@ -50,22 +49,15 @@ class MozJSImplScope; */ class MozJSScriptEngine final : public mongo::ScriptEngine { public: - MozJSScriptEngine(bool disableLoadStored); + MozJSScriptEngine(ExecutionEnvironment env); ~MozJSScriptEngine() override; - mongo::Scope* createScope() override; - mongo::Scope* createScopeForCurrentThread(boost::optional jsHeapLimitMB) override; - void runTest() override {} bool utf8Ok() const override { return true; } - void interrupt(unsigned opId) override; - - void interruptAll() override; - void enableJIT(bool value) override; bool isJITEnabled() const override; @@ -81,15 +73,21 @@ public: void registerOperation(OperationContext* ctx, MozJSImplScope* scope); void unregisterOperation(unsigned int opId); - using ScopeCallback = void (*)(Scope&); - ScopeCallback getScopeInitCallback() { - return _scopeInitCallback; - }; + void interrupt(unsigned opId) override; + void interruptAll() override; DeadlineMonitor& getDeadlineMonitor() { return _deadlineMonitor; } + ExecutionEnvironment executionEnvironment() const { + return _executionEnvironment; + } + +protected: + mongo::Scope* createScope() override; + mongo::Scope* createScopeForCurrentThread(boost::optional jsHeapLimitMB) override; + private: /** * This mutex protects _opToScopeMap @@ -101,6 +99,7 @@ private: // _globalInterruptLock). DeadlineMonitor _deadlineMonitor; + ExecutionEnvironment _executionEnvironment; std::string _loadPath; }; diff --git a/src/mongo/scripting/mozjs/implscope.cpp b/src/mongo/scripting/mozjs/implscope.cpp index e3115992fca..8060e614c03 100644 --- a/src/mongo/scripting/mozjs/implscope.cpp +++ b/src/mongo/scripting/mozjs/implscope.cpp @@ -526,6 +526,13 @@ MozJSImplScope::MozJSImplScope(MozJSScriptEngine* engine, boost::optional j execSetup(JSFiles::assert); execSetup(JSFiles::types); + if (_engine->executionEnvironment() == ExecutionEnvironment::Server) { + // For legacy support in server-side javascript execution, delete the ECMAScript defined + // `Map` type and replace it with our `BSONAwareMap` implementation. + ObjectWrapper(_context, _global).deleteProperty("Map"); + ObjectWrapper(_context, _global).renameAndDeleteProperty("BSONAwareMap", "Map"); + } + // install global utility functions installGlobalUtils(*this); _mongoHelpersProto.install(_global); @@ -1110,10 +1117,6 @@ void MozJSImplScope::installBSONTypes() { _timestampProto.install(_global); _uriProto.install(_global); _statusProto.install(_global); - - // This builtin map is a javascript 6 thing. We want our version. so - // take theirs out - ObjectWrapper(_context, _global).deleteProperty("Map"); } void MozJSImplScope::installDBAccess() { diff --git a/src/mongo/scripting/mozjs/module_loader_test.cpp b/src/mongo/scripting/mozjs/module_loader_test.cpp index c6611278dd2..d7a526a9501 100644 --- a/src/mongo/scripting/mozjs/module_loader_test.cpp +++ b/src/mongo/scripting/mozjs/module_loader_test.cpp @@ -45,7 +45,7 @@ namespace mongo { namespace mozjs { TEST(ModuleLoaderTest, ImportBaseSpecifierFails) { - mongo::ScriptEngine::setup(); + mongo::ScriptEngine::setup(ExecutionEnvironment::TestRunner); std::unique_ptr scope(mongo::getGlobalScriptEngine()->newScope()); auto code = "import * as test from \"base_specifier\""_sd; @@ -61,7 +61,7 @@ TEST(ModuleLoaderTest, ImportBaseSpecifierFails) { #if !defined(_WIN32) TEST(ModuleLoaderTest, ImportDirectoryFails) { - mongo::ScriptEngine::setup(); + mongo::ScriptEngine::setup(ExecutionEnvironment::TestRunner); std::unique_ptr scope(mongo::getGlobalScriptEngine()->newScope()); auto code = fmt::format("import * as test from \"{}\"", @@ -78,7 +78,7 @@ TEST(ModuleLoaderTest, ImportDirectoryFails) { #endif TEST(ModuleLoaderTest, ImportInInteractiveFails) { - mongo::ScriptEngine::setup(); + mongo::ScriptEngine::setup(ExecutionEnvironment::TestRunner); std::unique_ptr scope(mongo::getGlobalScriptEngine()->newScope()); auto code = "import * as test from \"some_module\""_sd; @@ -96,7 +96,7 @@ TEST(ModuleLoaderTest, ImportInInteractiveFails) { } TEST(ModuleLoaderTest, TopLevelAwaitWorks) { - mongo::ScriptEngine::setup(); + mongo::ScriptEngine::setup(ExecutionEnvironment::TestRunner); std::unique_ptr scope(mongo::getGlobalScriptEngine()->newScope()); auto code = "async function test() { return 42; } await test();"_sd; ASSERT_DOES_NOT_THROW(scope->exec(code, diff --git a/src/mongo/scripting/mozjs/objectwrapper.cpp b/src/mongo/scripting/mozjs/objectwrapper.cpp index 40d5de6cbea..5ad0cf5a170 100644 --- a/src/mongo/scripting/mozjs/objectwrapper.cpp +++ b/src/mongo/scripting/mozjs/objectwrapper.cpp @@ -546,6 +546,13 @@ void ObjectWrapper::rename(Key from, const char* to) { setValue(from, undefValue); } +void ObjectWrapper::renameAndDeleteProperty(Key from, const char* to) { + JS::RootedValue value(_context); + getValue(from, &value); + setValue(to, value); + from.del(_context, _object); +} + bool ObjectWrapper::hasField(Key key) { return key.has(_context, _object); } diff --git a/src/mongo/scripting/mozjs/objectwrapper.h b/src/mongo/scripting/mozjs/objectwrapper.h index d4e19c15abe..d5ff582708e 100644 --- a/src/mongo/scripting/mozjs/objectwrapper.h +++ b/src/mongo/scripting/mozjs/objectwrapper.h @@ -164,6 +164,9 @@ public: void rename(Key key, const char* to); + // Rename key and delete the original property + void renameAndDeleteProperty(Key key, const char* to); + // has field walks the prototype heirarchy bool hasField(Key key); diff --git a/src/mongo/shell/db.js b/src/mongo/shell/db.js index f6252755a98..ad3345150d9 100644 --- a/src/mongo/shell/db.js +++ b/src/mongo/shell/db.js @@ -617,7 +617,7 @@ DB.prototype.groupeval = function(parmsObj) { var groupFunction = function() { var parms = args[0]; // eslint-disable-line var c = globalThis.db[parms.ns].find(parms.cond || {}); - var map = new Map(); + var map = new BSONAwareMap(); var pks = parms.key ? Object.keySet(parms.key) : null; var pkl = pks ? pks.length : 0; var key = {}; diff --git a/src/mongo/shell/mongo_main.cpp b/src/mongo/shell/mongo_main.cpp index 4f20812ac68..65c579e592d 100644 --- a/src/mongo/shell/mongo_main.cpp +++ b/src/mongo/shell/mongo_main.cpp @@ -923,7 +923,7 @@ int mongo_main(int argc, char* argv[]) { } mongo::ScriptEngine::setConnectCallback(mongo::shell_utils::onConnect); - mongo::ScriptEngine::setup(); + mongo::ScriptEngine::setup(ExecutionEnvironment::TestRunner); mongo::getGlobalScriptEngine()->setJSHeapLimitMB(shellGlobalParams.jsHeapLimitMB); mongo::getGlobalScriptEngine()->setScopeInitCallback(mongo::shell_utils::initScope); mongo::getGlobalScriptEngine()->enableJIT(!shellGlobalParams.nojit); diff --git a/src/mongo/shell/types.js b/src/mongo/shell/types.js index 64fcb624b50..7a007ddd0c0 100644 --- a/src/mongo/shell/types.js +++ b/src/mongo/shell/types.js @@ -546,15 +546,12 @@ if (typeof (BinData) != "undefined") { print("warning: no BinData class"); } -// Map -if (typeof (Map) == "undefined") { - // eslint-disable-next-line - Map = function() { - this._data = {}; - }; -} +// BSONAwareMap +BSONAwareMap = function() { + this._data = {}; +}; -Map.hash = function(val) { +BSONAwareMap.hash = function(val) { if (!val) return val; @@ -575,19 +572,19 @@ Map.hash = function(val) { throw Error("can't hash : " + typeof (val)); }; -Map.prototype.put = function(key, value) { +BSONAwareMap.prototype.put = function(key, value) { var o = this._get(key); var old = o.value; o.value = value; return old; }; -Map.prototype.get = function(key) { +BSONAwareMap.prototype.get = function(key) { return this._get(key).value; }; -Map.prototype._get = function(key) { - var h = Map.hash(key); +BSONAwareMap.prototype._get = function(key) { + var h = BSONAwareMap.hash(key); var a = this._data[h]; if (!a) { a = []; @@ -603,7 +600,7 @@ Map.prototype._get = function(key) { return o; }; -Map.prototype.values = function() { +BSONAwareMap.prototype.values = function() { var all = []; for (var k in this._data) { this._data[k].forEach(function(z) {