[m-rev.] for review: Delete closeable_channel.is_closed.
novalazy at gmail.com
Fri Apr 17 17:04:05 AEST 2020
This is a plausible sequence of events leading to a failure in the
hard_coded/closeable_channel_test.m test case:
thread A thread B
Thread B tries to take the next item from the channel, during which time
it holds the Full semaphore of the from the mvar at the read end of the
Thread A attempts to verify that the channel is really closed.
It tries to read from the mvar at the write end of the channel, which is
the same mvar as thread B is reading from (because the channel is empty).
Thread A cannot acquire the Full semaphore of the mvar as it is being
held by thread B, resulting in the call is_closed(Chan, IsClosed) to
return with IsClosed = no.
Delete the is_closed predicate. It was of questionable utility
Delete call to is_closed in test case.
library/thread.closeable_channel.m | 32 +++--------------------
tests/hard_coded/closeable_channel_test.m | 9 -------
2 files changed, 3 insertions(+), 38 deletions(-)
diff --git a/library/thread.closeable_channel.m b/library/thread.closeable_channel.m
index 32b906751..7b9677d83 100644
@@ -39,17 +39,6 @@
:- pred close(closeable_channel(T)::in, io::di, io::uo) is det.
- % Return the state of a channel at a point in time. If the return value
- % is `yes' then the channel is closed, and will remain closed.
- % If the return value is `no' then the channel was not yet closed,
- % but by the time you inspect the value the channel could be closed by
- % another thread.
- % We (the Mercury developers) are unsure if this predicate will be useful
- % in practice. If you do find a use for it, please let us know.
-:- pred is_closed(closeable_channel(T)::in, bool::out, io::di, io::uo) is det.
:- type take_result(T)
@@ -127,27 +116,12 @@ put(channel(_Read, Write), Val, Success, !IO) :-
close(channel(_Read, Write), !IO) :-
mvar.take(Write, Hole, !IO),
+ % We have exclusive WRITE access to the hole.
+ % If the channel is open, the hole is empty so this will succeed.
+ % If the channel is closed, the hole is already full with `closed'.
mvar.try_put(Hole, closed, _Success, !IO),
mvar.put(Write, Hole, !IO).
-is_closed(channel(_Read, Write), IsClosed, !IO) :-
- mvar.take(Write, Hole, !IO),
- mvar.try_read(Hole, HoleFull, !IO),
- HoleFull = yes(ItemOrClosed),
- ItemOrClosed = closed,
- IsClosed = yes
- ItemOrClosed = item(_, _),
- unexpected($pred, "open channel has full hole at write end")
- HoleFull = no,
- IsClosed = no
- mvar.put(Write, Hole, !IO).
take(channel(Read, _Write), Res, !IO) :-
mvar.take(Read, Head, !IO),
mvar.read(Head, ItemOrClosed, !IO),
diff --git a/tests/hard_coded/closeable_channel_test.m b/tests/hard_coded/closeable_channel_test.m
index cf747c7e0..2a8ebc1cf 100644
@@ -75,15 +75,6 @@ run_test(!IO) :-
% Close open channel.
trace_close(Log, InCh, !IO),
- % Check is_closed.
- is_closed(InCh, IsClosed, !IO),
- IsClosed = yes
- IsClosed = no,
- log(Log, "ERROR: channel not closed", !IO)
% Wait for threads to exit.
log(Log, "waiting for worker threads to exit", !IO),
list.foldl(semaphore.wait, DoneSems, !IO),
More information about the reviews