Skip to content

h2o_spawnp mapped fds closes wrong fd #1203

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

Closed
wants to merge 2 commits into from

Conversation

deweerdt
Copy link
Member

It's possible that the mapped fds have the same value (in particular for
fastcgi, where the target fd is number 5). In that case, we end up
doing:

    dup2(5, 5);
    close(5);

ending up with no valid fd 5. This patch prevents the dup if the mapped
fd already has the expected value.

It's possible that the mapped fds have the same value (in particular for
fastcgi, where the target fd is number 5). In that case, we end up
doing:
    dup2(5, 5);
    close(5);

ending up with no valid fd 5. This patch prevents the dup if the mapped
fd already has the expected value.
@kazuho
Copy link
Member

kazuho commented Feb 20, 2017

Thank you for noticing the issue!

Could you please check if the suggested solution works for inputs like: { 3, 1, 1, 2, -1}?

I think we'd need to take care of such patterns (the source fd (in the case of the example 1) appears in one of the preceding destination fds).

Also handle the case where the mapped fd destination is found in the list of
source fds to map later.
@deweerdt
Copy link
Member Author

Could you please check if the suggested solution works for inputs like: { 3, 1, 1, 2, -1}?

Handled the case in 4cbf9bb, thanks for pointing it out.

@kazuho
Copy link
Member

kazuho commented Feb 22, 2017

I'm very sorry but it seems that we don't actually need 4cbf9bb, since posix_spawnp is defined to apply the file actions in the order they appear.

The file actions specified by the spawn file actions object shall be performed in the order in which they were added to the spawn file actions object.
posix_spawn - opengroup.org

I will cherry-pick 817da3e once the CI succeeds.

kazuho pushed a commit that referenced this pull request Feb 22, 2017
It's possible that the mapped fds have the same value (in particular for
fastcgi, where the target fd is number 5). In that case, we end up
doing:
    dup2(5, 5);
    close(5);

ending up with no valid fd 5. This patch prevents the dup if the mapped
fd already has the expected value.
@kazuho kazuho closed this Feb 22, 2017
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