-
Notifications
You must be signed in to change notification settings - Fork 78
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
fix timeout of age_callout_queue #32
base: master
Are you sure you want to change the base?
Conversation
@@ -338,7 +338,7 @@ void igmpProxyRun(void) { | |||
* call gettimeofday. | |||
*/ | |||
if (Rt == 0) { | |||
curtime.tv_sec = lasttime.tv_sec + secs; | |||
curtime.tv_sec = lasttime.tv_sec + timeout->tv_sec;//timeout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC updating timeout
value of pselect is Linux specific. So it would not work on other platforms correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really manpage for pselect says:
On Linux, select() modifies timeout to reflect the amount of time not slept; most other implementations do not do this. (POSIX.1 permits either behavior.) This causes problems both when Linux code which reads timeout is ported to other operating systems, and when code is ported to Linux that reuses a struct timeval for multiple select()s in a loop without reinitializing it. Consider timeout to be undefined after select() returns.
So this patch is Linux-specific, but igmpproxy is used on more *BSD platforms.
@@ -338,7 +338,7 @@ void igmpProxyRun(void) { | |||
* call gettimeofday. | |||
*/ | |||
if (Rt == 0) { | |||
curtime.tv_sec = lasttime.tv_sec + secs; | |||
curtime.tv_sec = lasttime.tv_sec + timeout->tv_sec;//timeout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really manpage for pselect says:
On Linux, select() modifies timeout to reflect the amount of time not slept; most other implementations do not do this. (POSIX.1 permits either behavior.) This causes problems both when Linux code which reads timeout is ported to other operating systems, and when code is ported to Linux that reuses a struct timeval for multiple select()s in a loop without reinitializing it. Consider timeout to be undefined after select() returns.
So this patch is Linux-specific, but igmpproxy is used on more *BSD platforms.
I have found that issue too and fixed it like in this PR. On FreeBSD, this patch works like expected. I think this is the same as in #59 . When the solution there works like expected, I think we should probably use it over this because of portability. |
I guess that instead of using undefined value |
timeout->tv_sec = (secs > 3) ? 3 : secs; // aimwang: set max timeout
pselect return max timeout is 3 seconds not secs when Rt==0