Skip to content

Commit dc56702

Browse files
committed
Trap on call to {stream,future}.cancel-{read,write} after a prior cancellation blocked
1 parent d336ed6 commit dc56702

3 files changed

Lines changed: 30 additions & 10 deletions

File tree

design/mvp/CanonicalABI.md

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1523,7 +1523,8 @@ class CopyState(Enum):
15231523
IDLE = 1
15241524
SYNC_COPYING = 2
15251525
ASYNC_COPYING = 3
1526-
DONE = 4
1526+
CANCELLING_COPY = 4
1527+
DONE = 5
15271528

15281529
class CopyEnd(Waitable):
15291530
state: CopyState
@@ -1533,18 +1534,23 @@ class CopyEnd(Waitable):
15331534
self.state = CopyState.IDLE
15341535

15351536
def copying(self):
1536-
return self.state == CopyState.SYNC_COPYING or self.state == CopyState.ASYNC_COPYING
1537+
match self.state:
1538+
case CopyState.IDLE | CopyState.DONE:
1539+
return False
1540+
case CopyState.SYNC_COPYING | CopyState.ASYNC_COPYING | CopyState.CANCELLING_COPY:
1541+
return True
1542+
assert(False)
15371543

15381544
def drop(self):
15391545
trap_if(self.copying())
15401546
Waitable.drop(self)
15411547
```
15421548
As shown in `drop`, attempting to drop a readable or writable end while a copy
1543-
is in progress traps. This means that client code must take care to wait for
1544-
these operations to finish (potentially cancelling them via
1545-
`stream.cancel-{read,write}`) before dropping. The `SYNC_COPY` vs. `ASYNC_COPY`
1546-
distinction is tracked in the state to determine whether the copy operation can
1547-
be cancelled.
1549+
is in progress or in the process of being cancelled traps. This means that
1550+
client code must take care to wait for these operations to finish (potentially
1551+
cancelling them via `stream.cancel-{read,write}`) before dropping. The
1552+
`SYNC_COPY` vs. `ASYNC_COPY` distinction is tracked in the state to determine
1553+
whether the copy operation can be cancelled.
15481554

15491555
Given the above, we can define the concrete `{Readable,Writable}StreamEnd`
15501556
classes which are almost entirely symmetric, with the only difference being
@@ -4305,6 +4311,7 @@ def cancel_copy(EndT, event_code, stream_or_future_t, async_, thread, i):
43054311
trap_if(not isinstance(e, EndT))
43064312
trap_if(e.shared.t != stream_or_future_t.t)
43074313
trap_if(e.state != CopyState.ASYNC_COPYING)
4314+
e.state = CopyState.CANCELLING_COPY
43084315
if not e.has_pending_event():
43094316
e.shared.cancel()
43104317
if not e.has_pending_event():
@@ -4321,7 +4328,8 @@ unconditionally traps if it transitively attempts to make a synchronous call to
43214328
`cancel-read` or `cancel-write` (regardless of whether the cancellation would
43224329
have completed without blocking). There is also a trap if there is not
43234330
currently an async copy in progress (sync copies do not expect or check for
4324-
cancellation and thus cannot be cancelled).
4331+
cancellation and thus cannot be cancelled and repeatedly cancelling the same
4332+
async copy after the first call blocked is not allowed).
43254333

43264334
The *first* check for `e.has_pending_event()` catches the case where the copy has
43274335
already racily finished, in which case we must *not* call `cancel()`. Calling

design/mvp/canonical-abi/definitions.py

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -887,7 +887,8 @@ class CopyState(Enum):
887887
IDLE = 1
888888
SYNC_COPYING = 2
889889
ASYNC_COPYING = 3
890-
DONE = 4
890+
CANCELLING_COPY = 4
891+
DONE = 5
891892

892893
class CopyEnd(Waitable):
893894
state: CopyState
@@ -897,7 +898,12 @@ def __init__(self):
897898
self.state = CopyState.IDLE
898899

899900
def copying(self):
900-
return self.state == CopyState.SYNC_COPYING or self.state == CopyState.ASYNC_COPYING
901+
match self.state:
902+
case CopyState.IDLE | CopyState.DONE:
903+
return False
904+
case CopyState.SYNC_COPYING | CopyState.ASYNC_COPYING | CopyState.CANCELLING_COPY:
905+
return True
906+
assert(False)
901907

902908
def drop(self):
903909
trap_if(self.copying())
@@ -2471,6 +2477,7 @@ def cancel_copy(EndT, event_code, stream_or_future_t, async_, thread, i):
24712477
trap_if(not isinstance(e, EndT))
24722478
trap_if(e.shared.t != stream_or_future_t.t)
24732479
trap_if(e.state != CopyState.ASYNC_COPYING)
2480+
e.state = CopyState.CANCELLING_COPY
24742481
if not e.has_pending_event():
24752482
e.shared.cancel()
24762483
if not e.has_pending_event():

design/mvp/canonical-abi/run_tests.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2031,6 +2031,11 @@ def core_func(thread, args):
20312031
host_source.block_cancel()
20322032
[ret] = canon_stream_cancel_read(StreamType(U8Type()), True, thread, rsi)
20332033
assert(ret == definitions.BLOCKED)
2034+
try:
2035+
canon_stream_cancel_read(StreamType(U8Type()), True, thread, rsi)
2036+
assert(False)
2037+
except Trap:
2038+
pass
20342039
host_source.write([7,8])
20352040
host_source.unblock_cancel()
20362041
[seti] = canon_waitable_set_new(thread)

0 commit comments

Comments
 (0)