Skip to content

Conversation

@mwilck
Copy link

@mwilck mwilck commented Feb 7, 2019

Quit if systemd doesn't pass a valid socket, and no -s parameter
given. Otherwise fcgiwrap may try listening on stdin, with
bad results.

Basically, it loops endlessly printing error messages like this:

An error occurred while reading CGI reply (no response received)"
Cannot get script name, are DOCUMENT_ROOT and SCRIPT_NAME (or SCRIPT_FILENAME) set and is the script executable?
Status: 502 Bad Gateway
Content-type: text/plain

Quit if systemd doesn't pass a valid socket, and no -s parameter
given. Otherwise fcgiwrap may try listening on stdin, with
bad results.
@CameronNemo
Copy link

CameronNemo commented Apr 13, 2019

Sorry, but this does not seem to behave as you describe in your comment here. This will return whenever the -s flag is not passed.

How are services expected to check that libsystemd is not buggy?

@mwilck
Copy link
Author

mwilck commented Apr 17, 2019

You are right, it returns if -s is not passed - unless fcgiwrap is called by systemd, and systemd passes a valid socket file descriptor. I don't think libsystemd is buggy in this context. If it was, your system would hardly ever boot at all. But maybe I'd understand your problem better if you provided some more detail about how fcgiwrap is called in your scenario.

Blindly listening on stdin is wrong in any case, at least if fcgiwrap was started from a systemd service.

@CameronNemo
Copy link

CameronNemo commented Apr 17, 2019

WIth your patch, when systemd calls fcgiwrap, socket_url will be NULL and fcgiwrap will exit. I just noticed the conditionally compiled else if. Gross.

@mwilck
Copy link
Author

mwilck commented Apr 23, 2019

I agree it isn't beautiful, but that's the existing code. I was trying to keep the patch small and non-intrusive. @CameronNemo, do you have a suggestion for improvement?

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.

2 participants