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

improve ARM support #26

Closed
wants to merge 1 commit into from
Closed

improve ARM support #26

wants to merge 1 commit into from

Conversation

xrstf
Copy link
Contributor

@xrstf xrstf commented Aug 15, 2012

It seems that accept() is ignoring the "infinite timeout" parameter (0
or -1) on ARM. This makes it return instantly without an actual
connection. This kills the worker, which is restarted immediately. We
could just wait for let's say 1 hour and live with the fact that our
workers will be restarted again and again. But this fix is better.
We cannot use the (functioning!) socket API directly (without stream
support), because more stream functions are used later on that would
have to be replaced as well.

We replaced the stream_get_line() with fgets() to work around another
issue: When selecting and accepting the connection, the s_g_l() would
block for the default_socket_timeout duration before reading the request
and handling it.

…-blocking accept() call on ARM machines

It seems that accept() is ignoring the "infinite timeout" parameter (0
or -1) on ARM. This makes it return instantly without an actual
connection. This kills the worker, which is restarted immediately. We
could just wait for let's say 1 hour and live with the fact that our
workers will be restarted again and again. But this fix is better.
We cannot use the (functioning!) socket API directly (without stream
support), because more stream functions are used later on that would
have to be replaced as well.

We replaced the stream_get_line() with fgets() to work around another
issue: When selecting and accepting the connection, the s_g_l() would
block for the default_socket_timeout duration before reading the request
and handling it.
@indeyets
Copy link
Owner

for historical purposes: upstream bug

@indeyets
Copy link
Owner

@xrstf regarding second part of the patch. do you refer to #20? Because I thought that was already fixed by newer versions of php

@indeyets
Copy link
Owner

@xrstf I pushed first part of your patch (with minor editing). Please check if it works for you.

Regarding second part, I'm waiting for some comments from you

@xrstf
Copy link
Contributor Author

xrstf commented Aug 23, 2012

No, the second part of the patch (replacing stream_get_line with fgets) is unrelated to #20. We didn't test PHP < 3.5.11 and < 5.4 any further, since 5.3.10 was so broken.

I'll test your changes later. Can take some time though, since I don't own an ARM device myself.

@indeyets
Copy link
Owner

So, what is the scenario when this patch is needed? cause everything seems to work here just fine without it and I have default_socket_timeout=60

@xrstf
Copy link
Contributor Author

xrstf commented Sep 3, 2012

The patch is always needed when running on an ARM system. default_socket_timeout is unfortunately irrelevant in this case, as far as I remember.

@indeyets
Copy link
Owner

indeyets commented Sep 4, 2012

Well… Let's do it this way.

@xrstf Can you submit this as a separate issue? It looks like a core bug, so I am not in favour of "fixing" this on AiP side, actually. But I might change my mind.

And IMHO looks like it IS actually related to that old 5.3 bug.

Closing this one, waiting for the new one which will be strictly stream_get_line() related

@indeyets indeyets closed this Sep 4, 2012
@xrstf
Copy link
Contributor Author

xrstf commented Sep 5, 2012

I'm fine with that solution. :-)

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