Skip to content

Commit fccf7d1

Browse files
committed
Refactor Python definitions related to borrow handles
This commit is at attempt of mine to simplify the moving parts in how handles are specified by refactoring the related Python code which specifies how everything works. Previously I found the interactions between the various methods difficult to understand due to some layers of abstraction and where various methods were invoked. Here I've attempted to (subjectively at least) simplify a few things: * The `add` and `remove` methods on `HandleTable` no longer do any bookkeeping of borrow-related information. This makes `HandleTable` a pure "slab class" where it's ideally easy to see "oh hey it's a slab" and then ignore the bulk of the Python code which otherwise just says what a slab is. * The `add_borrow_to_table` and `remove_borrow_from_table` methods were removed in favor of "just" frobbing the `borrow_count` field directly. The method abstraction at least seemed to me to imply more management behind the scenes but it ended up only ever being an increment/decrement. * The `lift_borrow_from` method was renamed to `track_owning_lend` to indicate that it's not doing any lifting operations and it's part of the metadata tracking. * The metadata bookkeeping in `add` and `remove` is now inlined directly into the `{lift,lower}_{own,borrow}` and `canon_resource_drop` functions, along with the `add_borrow_to_table` and `remove_borrow_from_table` methods. Overall to me at least this felt easier to understand where there are seven basic primitives here: `resource.{new,rep,drop}` plus `{lift,lower}_{own,borrow}` and all seven now relatively clearly document, just by looking at their single method, what each primitive is responsible for. Previously methods had to be followed to understand how the borrowing/own dynamic tracking all interacted but it's, at least in theory, more up-front now.
1 parent d445b5a commit fccf7d1

File tree

2 files changed

+38
-43
lines changed

2 files changed

+38
-43
lines changed

design/mvp/CanonicalABI.md

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -237,22 +237,17 @@ for the component caller or callee, resp.
237237
The meaning of the `opts` and `inst` fields are described with their associated
238238
types below.
239239

240-
The `lenders` and `borrow_count` fields are used by the following 4 methods of
241-
`CallContext`. These methods are called at the appropriate points in the
242-
lifecycle of a call (below) and maintain the bookkeeping to dynamically ensure
243-
that `own` handles are not dropped while they have been `borrow`ed and that all
244-
`borrow` handles created for a call are dropped before the end of the call.
240+
The `lenders` and `borrow_count` fields are used by the following helper
241+
methods of `CallContext` plus the `{lift,lower}_{own,borrow}` operations. These
242+
fields are updated at the appropriate points in the lifecycle of a call (below)
243+
and maintain the bookkeeping to dynamically ensure that `own` handles are not
244+
dropped while they have been `borrow`ed and that all `borrow` handles created
245+
for a call are dropped before the end of the call.
245246
```python
246-
def lift_borrow_from(self, lending_handle):
247-
if lending_handle.own:
248-
lending_handle.lend_count += 1
249-
self.lenders.append(lending_handle)
250-
251-
def add_borrow_to_table(self):
252-
self.borrow_count += 1
253-
254-
def remove_borrow_from_table(self):
255-
self.borrow_count -= 1
247+
def track_owning_lend(self, lending_handle):
248+
assert(lending_handle.own)
249+
lending_handle.lend_count += 1
250+
self.lenders.append(lending_handle)
256251

257252
def exit_call(self):
258253
trap_if(self.borrow_count != 0)
@@ -380,23 +375,14 @@ free list in the free elements of `array`.
380375
else:
381376
i = len(self.array)
382377
self.array.append(h)
383-
if h.scope is not None:
384-
h.scope.add_borrow_to_table()
385378
return i
386379

387380
def remove(self, rt, i):
388381
h = self.get(i)
389-
trap_if(h.lend_count != 0)
390382
self.array[i] = None
391383
self.free.append(i)
392-
if h.scope is not None:
393-
h.scope.remove_borrow_from_table()
394384
return h
395385
```
396-
In addition to handling allocation of the handle elements, the `add` and
397-
`remove` methods make balanced calls to the `add_borrow_to_table` and
398-
`remove_borrow_from_table` methods of `CallContext` which participate in the
399-
enforcement of the dynamic borrow rules.
400386

401387
Finally, we can define `HandleTables` (plural) as simply a wrapper around
402388
a mutable mapping from `ResourceType` to `HandleTable`:
@@ -637,10 +623,13 @@ def unpack_flags_from_int(i, labels):
637623

638624
`own` handles are lifted by removing the handle from the current component
639625
instance's handle table, so that ownership is *transferred* to the lowering
640-
component.
626+
component. The lifting operation fails if unique ownership of the handle isn't
627+
possible, for example if the index was actually a `borrow` or if the `own`
628+
handle is currently being lent out as borrows.
641629
```python
642630
def lift_own(cx, i, t):
643631
h = cx.inst.handles.remove(t.rt, i)
632+
trap_if(h.lend_count != 0)
644633
trap_if(not h.own)
645634
return h.rep
646635
```
@@ -657,10 +646,11 @@ component instance's handle table:
657646
```python
658647
def lift_borrow(cx, i, t):
659648
h = cx.inst.handles.get(t.rt, i)
660-
cx.lift_borrow_from(h)
649+
if h.own:
650+
cx.track_owning_lend(h)
661651
return h.rep
662652
```
663-
The `lift_borrow_from` call to `CallContext` participates in the enforcement of
653+
The `track_owning_lend` call to `CallContext` participates in the enforcement of
664654
the dynamic borrow rules.
665655

666656

@@ -1013,6 +1003,7 @@ def lower_borrow(cx, rep, t):
10131003
if cx.inst is t.rt.impl:
10141004
return rep
10151005
h = HandleElem(rep, own=False, scope=cx)
1006+
cx.borrow_count += 1
10161007
return cx.inst.handles.add(t.rt, h)
10171008
```
10181009
The special case in `lower_borrow` is an optimization, recognizing that, when
@@ -1610,9 +1601,15 @@ the resource's destructor.
16101601
def canon_resource_drop(inst, rt, i):
16111602
h = inst.handles.remove(rt, i)
16121603
if h.own:
1604+
assert(h.scope is None)
1605+
trap_if(h.lend_count != 0)
16131606
trap_if(inst is not rt.impl and not rt.impl.may_enter)
16141607
if rt.dtor:
16151608
rt.dtor(h.rep)
1609+
else:
1610+
assert(h.scope is not None)
1611+
assert(h.scope.borrow_count > 0)
1612+
h.scope.borrow_count -= 1
16161613
```
16171614
The `may_enter` guard ensures the non-reentrance [component invariant], since
16181615
a destructor call is analogous to a call to an export.

design/mvp/canonical-abi/definitions.py

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -288,16 +288,10 @@ def __init__(self, opts, inst):
288288
self.lenders = []
289289
self.borrow_count = 0
290290

291-
def lift_borrow_from(self, lending_handle):
292-
if lending_handle.own:
293-
lending_handle.lend_count += 1
294-
self.lenders.append(lending_handle)
295-
296-
def add_borrow_to_table(self):
297-
self.borrow_count += 1
298-
299-
def remove_borrow_from_table(self):
300-
self.borrow_count -= 1
291+
def track_owning_lend(self, lending_handle):
292+
assert(lending_handle.own)
293+
lending_handle.lend_count += 1
294+
self.lenders.append(lending_handle)
301295

302296
def exit_call(self):
303297
trap_if(self.borrow_count != 0)
@@ -361,17 +355,12 @@ def add(self, h):
361355
else:
362356
i = len(self.array)
363357
self.array.append(h)
364-
if h.scope is not None:
365-
h.scope.add_borrow_to_table()
366358
return i
367359

368360
def remove(self, rt, i):
369361
h = self.get(i)
370-
trap_if(h.lend_count != 0)
371362
self.array[i] = None
372363
self.free.append(i)
373-
if h.scope is not None:
374-
h.scope.remove_borrow_from_table()
375364
return h
376365

377366
class HandleTables:
@@ -544,12 +533,14 @@ def unpack_flags_from_int(i, labels):
544533

545534
def lift_own(cx, i, t):
546535
h = cx.inst.handles.remove(t.rt, i)
536+
trap_if(h.lend_count != 0)
547537
trap_if(not h.own)
548538
return h.rep
549539

550540
def lift_borrow(cx, i, t):
551541
h = cx.inst.handles.get(t.rt, i)
552-
cx.lift_borrow_from(h)
542+
if h.own:
543+
cx.track_owning_lend(h)
553544
return h.rep
554545

555546
### Storing
@@ -795,6 +786,7 @@ def lower_borrow(cx, rep, t):
795786
if cx.inst is t.rt.impl:
796787
return rep
797788
h = HandleElem(rep, own=False, scope=cx)
789+
cx.borrow_count += 1
798790
return cx.inst.handles.add(t.rt, h)
799791

800792
### Flattening
@@ -1130,9 +1122,15 @@ def canon_resource_new(inst, rt, rep):
11301122
def canon_resource_drop(inst, rt, i):
11311123
h = inst.handles.remove(rt, i)
11321124
if h.own:
1125+
assert(h.scope is None)
1126+
trap_if(h.lend_count != 0)
11331127
trap_if(inst is not rt.impl and not rt.impl.may_enter)
11341128
if rt.dtor:
11351129
rt.dtor(h.rep)
1130+
else:
1131+
assert(h.scope is not None)
1132+
assert(h.scope.borrow_count > 0)
1133+
h.scope.borrow_count -= 1
11361134

11371135
### `canon resource.rep`
11381136

0 commit comments

Comments
 (0)