[m-rev.] for review: Delete closeable_channel.is_closed.
Peter Wang
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
-------- --------
close(Chan)
channel.take(Chan)
-> mvar.read(Head)
-> semaphore.wait(Full)
is_closed(Chan, IsClosed)
-> mvar.try_read(Hole)
-> semaphore.try_wait(Full)
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
channel.
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.
library/thread.closeable_channel.m:
Delete the is_closed predicate. It was of questionable utility
anyway.
tests/hard_coded/closeable_channel_test.m:
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
--- a/library/thread.closeable_channel.m
+++ b/library/thread.closeable_channel.m
@@ -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)
---> ok(T)
; closed.
@@ -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
--- a/tests/hard_coded/closeable_channel_test.m
+++ b/tests/hard_coded/closeable_channel_test.m
@@ -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),
--
2.25.0
More information about the reviews
mailing list