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

Redirect stdout and stderr to /dev/null if possible #143

Merged

Conversation

artyom-smirnov
Copy link
Contributor

Guardian closes stdin/stdout/stderr/... before
launching server binary so server can reuse those
descriptors when opening files or using shmem for
example. So some stray stdout can introduce
unwanted data or even crash server. Server do not
emit any stdout/stderr in release build but
UDRs or external libraries can print to stdout.

{
dup2(dev_null, fileno(stdout));
dup2(dev_null, fileno(stderr));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if dev_null is 1 or 2? What will do dup2(1, 1)?
If dev_null is not 1 or 2 - should it be closed after dup2 calls?
May be let users put stdout/stderr to some not-null file - for example as a tool to debug UDR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if dev_null is 1 or 2? What will do dup2(1, 1)?
If dev_null is not 1 or 2 - should it be closed after dup2 calls?

Will fix it.

May be let users put stdout/stderr to some not-null file - for example as a tool to debug UDR?

Originally this commit intended to fix *nix only problem caused by guardian. If we want to use output redirecting as some debugging tool it should be more unified and work on all platforms and without guardian. May be then server binary should have ability to redirect its output to /dev/null or any other file from config?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexPeshkoff I have updated branch. Please review. I have added config option to allow user define path. Also both classic and super now will behave same way when handling stdout/stderr. Changes are only for posix build for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For posix everything appears good.

Before these changes guardian and server binary closed
stdin/stdout/stderr/... and server can reuse those
descriptors when opening files or using shmem for
example. So some stray stdout can introduce
unwanted data or even crash server. Server do not
emit any stdout/stderr in release build but
UDRs or external libraries can print to stdout.

After these changes stdout/stderr will be kept opened
and user have options to redirect it to /dev/null (by default)
or other file by choise or just allow server to print.

New config option OutputRedirectionFile was introduced to
allow user control server behavior.
@AlexPeshkoff AlexPeshkoff merged commit 21b42b2 into FirebirdSQL:master Mar 21, 2018
@artyom-smirnov artyom-smirnov deleted the server_stdout_devnull_master branch March 28, 2018 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants