Skip to content

Commit

Permalink
Tweak latest post
Browse files Browse the repository at this point in the history
  • Loading branch information
@bpcreech
bpcreech committed Apr 9, 2024
1 parent bf62cf2 commit 80a4a5d
Showing 1 changed file with 47 additions and 29 deletions.
76 changes: 47 additions & 29 deletions content/post/mini-racer-v0.11.1.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 | |
Expand Down Expand Up @@ -443,6 +445,8 @@ deletion happens:
Then user code doesn't have to remember to free things at all! _What could go
possibly wrong?_

<center><div style="max-width: 300px;">

```goat
+--------------+ +--------------+
| +--->| |
Expand All @@ -451,6 +455,8 @@ deletion happens:
+--------------+ +--------------+
```

</div></center>

#### The trouble with finalizers

_What could possibly go wrong_ is that finalizers are called somewhat lazily by
Expand All @@ -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_.

<center><div style="text-align: center; max-width: 400px;">

```goat
+--------------+ +--------------+
| +--->| |
| A | | B |
| |<---+ |
+-------+------+ +-------+------+
Python space | __del__ | __del__
~~~~~~~~~~~~~~|~~~~~~~~~~~~~~~~~~~|~~~~~~~~~~~~~~
Python space | `__del__` | `__del__`
··············|···················|·············
C++ space | |
v v
+--------------+ +--------------+
Expand All @@ -487,6 +495,8 @@ C++ space | |
+--------------+ +--------------+
```

</div></center>

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
Expand All @@ -496,24 +506,26 @@ 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.

<center><div style="text-align: center; max-width: 600px;">

```goat
+------------------------------+ +--------------------------+
| +--->| |
| _ValueHandle | | _Context |
| |<---+ |
+----------+-------------------+ +---+----------------------+
| |
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
| +--------------------------+
| | |
Expand All @@ -522,6 +534,8 @@ C++ space | (calls mr_value_free | (calls mr_context_free
+--------------------------+
```

</div></center>

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
Expand Down Expand Up @@ -606,53 +620,57 @@ 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):

<center><div style="text-align: center; max-width: 600px;">

```goat
+---------------------------------+ +------------------------------+
| +--->| |
| _ValueHandle | | _Context |
| |<---+ |
+------------------------------+--+ +---+--------------------------+
| |
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) |
+---------------------------------+ |
| | |
| MiniRacer::BinaryValue | |
| | |
+----------------+----------------+ |(owns)
| (points into) v
+----------------+----------------+ | owns
| points into v
| +--------------------------+
| | |
+----------------->| v8::Isolate |
| |
+--------------------------+
```

</div></center>

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:
Expand Down

0 comments on commit 80a4a5d

Please sign in to comment.