diff --git a/content/post/mini-racer-v0.11.1.md b/content/post/mini-racer-v0.11.1.md index 532248e..1e070c3 100644 --- a/content/post/mini-racer-v0.11.1.md +++ b/content/post/mini-racer-v0.11.1.md @@ -301,18 +301,20 @@ The resulting setup looks roughly like this: | | 1. creates, ----->| | | | | | | v8::Isolate | | | | | 2. exposes, | | | | -| v8::Isolate* <-------------------------+ | | | +| v8::Isolate* <----+--------------------+ | | | | ^ | +--+----------+ | | -| | 6. enqueues | | ^ | | -| | | 3. ... then runs: `-' PumpMessages | | -| | | (looping until shutdown) | | +| | 6. enqueues | | ^ | | +| | | | | | | +| | | 3. … then runs '-' | | +| | | v8::Platform::PumpMessages | | +| | | (looping until shutdown) | | | | '---------------------------------------------' | | | | -+-----|--------------------------------------------------------------+ ++-----+--------------------------------------------------------------+ | 5. MiniRacer::IsolateManager | ::Run(task) | -+--------------------------+ +-----------------------+ ++-----+--------------------+ +-----------------------+ | | | | | MiniRacer::CodeEvaluator +----------->| MiniRacer::AdHocTask | | (etc) | 4. creates | | @@ -443,6 +445,8 @@ deletion happens: Then user code doesn't have to remember to free things at all! _What could go possibly wrong?_ +
+ ```goat +--------------+ +--------------+ | +--->| | @@ -451,6 +455,8 @@ deletion happens: +--------------+ +--------------+ ``` +
+ #### The trouble with finalizers _What could possibly go wrong_ is that finalizers are called somewhat lazily by @@ -470,14 +476,16 @@ finalizers for both objects, in _whatever_ order you like, and you declare it to be programmer error if these objects refer to each other in their finalizers, and you generally _declare that finalization order doesn't matter_. +
+ ```goat +--------------+ +--------------+ | +--->| | | A | | B | | |<---+ | +-------+------+ +-------+------+ -Python space | __del__ | __del__ -~~~~~~~~~~~~~~|~~~~~~~~~~~~~~~~~~~|~~~~~~~~~~~~~~ +Python space | `__del__` | `__del__` +··············|···················|············· C++ space | | v v +--------------+ +--------------+ @@ -487,6 +495,8 @@ C++ space | | +--------------+ +--------------+ ``` +
+ Unfortunately, _order sometimes matters_. Let's say those objects `A` and `B` are each managing C++ objects, `C` and `D`, respectively, as depicted above. Obviously, in the above picture, we should tear down `C` before `D` so we don't @@ -496,6 +506,8 @@ the link between C++ objects `C` and `D`. It is likely to call `B`'s finalizer first, tearing down `D` before `C`, thus leaving a dangling reference on the C++ side. +
+ ```goat +------------------------------+ +--------------------------+ | +--->| | @@ -503,17 +515,17 @@ side. | |<---+ | +----------+-------------------+ +---+----------------------+ | | -Python space | __del__ | __del__ -~~~~~~~~~~~~~~|~~~~~~~~~~~~~~~~~~~~~~~~~~~~|~~~~~~~~~~~~~~~~~~~~~~~~~ -C++ space | (calls mr_value_free | (calls mr_context_free - | with a pointer) | with a pointer) +Python space | `__del__` | `__del__` +··············|····························|···························· +C++ space | calls mr_value_free | calls mr_context_free + | with a pointer | with a pointer v v +------------------------------+ +--------------------------+ | | | | | MiniRacer::BinaryValue | | MiniRacer::Context | | | | | +----------------+-------------+ +------------+-------------+ - | (points into) | (owns) + | points into | owns | v | +--------------------------+ | | | @@ -522,6 +534,8 @@ C++ space | (calls mr_value_free | (calls mr_context_free +--------------------------+ ``` +
+ This happens in practice in MiniRacer: a Python `_Context` wraps a `MiniRacer::Context`, and a Python `_ValueHandle` wraps a `MiniRacer::BinaryValue`. But within C++ land, that `MiniRacer::Context` is what @@ -606,6 +620,8 @@ one pre-existing leak this way!) The resulting setup, just looking at Contexts and JS Values, sort of looks like the following diagram (and similar goes for async task handles): +
+ ```goat +---------------------------------+ +------------------------------+ | +--->| | @@ -613,30 +629,30 @@ the following diagram (and similar goes for async task handles): | |<---+ | +------------------------------+--+ +---+--------------------------+ | | -Python space __del__ | | __del__ -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~|~~~~~~~~~~~|~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -C++ space (calls mr_value_free | | (calls mr_context_free with - with a context ID + | | a uint64 id) - value handle pointer) | | +Python space `__del__` | | `__del__` +··································|···········|······························ +C++ space calls mr_value_free | | calls mr_context_free with + with a context ID + | | a uint64 id + value handle pointer | | | v | +-----------------------------+ +------>| | | MiniRacer::ContextFactory | - | (validates context IDs...) | + | validates context IDs… | +-----------+-----------------+ - | (...and passes valid - v calls to...) + | … and passes valid + v calls to … +------------------------------+ | | - +---------------------------| MiniRacer::Context | - | (passes individual value | | - | deletions to, and/or +----------+-------------------+ - | deletes in bulk upon | - v context teardown) | + +---------------------------+ MiniRacer::Context | + | "passes individual value | | + | deletions to, and/or +----------+-------------------+ + | deletes in bulk upon | + v context teardown" | +---------------------------------+ | | | | | MiniRacer::BinaryValueFactory | | - | (validates handle ptrs) | | + | validates handle ptrs | | +----+----------------------------+ | | delete (if handle is valid, or upon | v factory teardown if never deleted) | @@ -644,8 +660,8 @@ C++ space (calls mr_value_free | | (calls mr_context_free with | | | | MiniRacer::BinaryValue | | | | | - +----------------+----------------+ |(owns) - | (points into) v + +----------------+----------------+ | owns + | points into v | +--------------------------+ | | | +----------------->| v8::Isolate | @@ -653,6 +669,8 @@ C++ space (calls mr_value_free | | (calls mr_context_free with +--------------------------+ ``` +
+ With this new setup, if Python finalizes ("calls `__del__` on") the `_Context` _before_ a `_ValueHandle` that belonged to that context, aka "the wrong order", what happens now is: