Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

captureOutput(): Protect against unbalanced sinks #126

Open
HenrikBengtsson opened this issue Nov 18, 2020 · 0 comments
Open

captureOutput(): Protect against unbalanced sinks #126

HenrikBengtsson opened this issue Nov 18, 2020 · 0 comments

Comments

@HenrikBengtsson
Copy link
Owner

HenrikBengtsson commented Nov 18, 2020

Issue

utils::capture.output(expr) and therefore also R.utils::captureOutput(expr) assumes that the evaluated expression expr closes any sinks it opens. However, if this is not the case, then utils::capture.output() fails to properly close the sink it opened itself and therefore also fail to capture the output. For example,

> bfr <- utils::capture.output({ cat("hello\n"); sink(tf <- tempfile()); cat("world\n") })

The above call corrupts R's sinks and their associated connections;

> print(bfr)
Error in print.default(bfr) : invalid connection

To fix this, we need to know how many sinks are still open and attempt to close them manually, e.g.

> sink()
Error in sink() : invalid connection
> bfr
[1] "hello"

Suggestion

  1. Have captureOutput() detect when the sink stack is unbalanced before attempting to close the sink it has opened. This can be done by recording the depth <- sink.number(type = type) after initiating its sink and then making sure that currDepth <- sink.number(type = type) matches this sink depth before closing the sink again. If currDepth != depth, then there is an imbalance. What should happen in this case could be controlled by an new argument onUnbalance = c("ignore", "error", "warning"), where "ignore" is the current behavior.

  2. If currDepth > depth, then captureOutput() could close the extra ones automatically before closing its own sink. This could be controlled by a new argument balance = FALSE. If TRUE, it'll close the extra sinks. If FALSE, the error message could include information on how many extra sinks are open.

  3. If currDepth < depth, then there is nothing captureOutput() can do. However, it could still detect it and exit cleanly, e.g. making sure to close any raw/file connection it has opened and generate an informative error message explaining it could not recover.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant