Fix various issues around close/1 and stream realiasing #2818
+380
−164
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…aren't set to Stdin and Stdout
These two fields are able to hold `Stream` instances, which predicates like `close/1` expect to be managed properly for their correctness. To ensure that this is the case, I have removed direct accesses to those two fields, so that they can be properly managed in one place.
Before this change, the following set of queries would behave incorrectly:
```
?- open("/tmp/out.log", write, S), set_output(S).
prints(""), write("/tmp/out.log", "S = stream(...)").
?- write(user_output, hello).
prints("hello"), unexpected.
prints(""), write("/tmp/out.log", "hello"). % Expected, but not found.
```
Now, `set_output/1` and `set_input/1` properly bind the `user_output` and
`user_input` aliases, making the queries above behave as expected.
adri326
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2806, namely the following issues I spotted around
close/1and'$set_stream_options'(which can currently be accessed throughopen(stream(S), _, _, NewOptions)); in order:current_output(S), close(S).would partially closeuser_outputif it was previously set to a stream other thanStdoutopen(stream(S), ..., [alias(new_alias)])would leave thestream_aliasesmap unsynchronized with the new option, causing issues whenclose(S)was calledopen(stream(S), ..., [alias(user_output)]), close(S)would overrideuser_outputand partially close it, makingwrite(user_output, hello)fail unexpectedlyset_output(S), write(user_output, hello)would not writehellointoSTo fix these issues and prevent similar one from appearing, I have encapsulated accesses to
machine.indices.streamandmachine.indices.stream_aliasesbehind a few methods and made direct accesses illegal in 0cf46d3 (the biggest commit of the bunch).The only remaining pain point is that
options_mut()can be used to break the relationship betweenstream.options().get_alias()andmachine.indices.stream_aliases. There's no easy way to enforce this statically other than changing the API for constructing new streams. I chose to lessen the complexity of this PR and instead leave a warning on the affected methods.I took care to address each issue in a different commit.