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

AmqpSettings IdleTimeout error in case you time travel #576

Closed
optimuserik opened this issue Dec 14, 2023 · 2 comments
Closed

AmqpSettings IdleTimeout error in case you time travel #576

optimuserik opened this issue Dec 14, 2023 · 2 comments

Comments

@optimuserik
Copy link

optimuserik commented Dec 14, 2023

AmqpSettings.IdleTimeout is set to int, but used as a uint

https://github.com/Azure/amqpnetlite/blob/3e42f0cc2e2981fd5c8376ef3338aa82b433eafe/src/Net/AmqpSettings.cs#L74C20-L74C31

public int IdleTimeout
        {
            get;
            set;
        }

https://github.com/Azure/amqpnetlite/blob/3e42f0cc2e2981fd5c8376ef3338aa82b433eafe/src/Connection.cs#L243C25-L243C25

IdleTimeOut = (uint)amqpSettings.IdleTimeout / 2

We may want to change the BIOS time, and if we set it to a past time this will cause issues.

When checking elapsed time

uint elapsed = (uint)((now.Ticks - last.Ticks) / Encoder.TicksPerMillisecond);

                    uint elapsed = (uint)((now.Ticks - last.Ticks) / Encoder.TicksPerMillisecond);

the substraction can go into negative, since you time traveled and it will be a very high uint number.

Since IdleTimeout is by default set to int.MaxValue

https://github.com/Azure/amqpnetlite/blob/3e42f0cc2e2981fd5c8376ef3338aa82b433eafe/src/Net/ConnectionFactoryBase.cs#L45C17-L45C28

                IdleTimeout = int.MaxValue,

which presumably is intended to mean that it 'never' times out ( which would be what we want).

However the previously mentioned

                    uint elapsed = (uint)((now.Ticks - last.Ticks) / Encoder.TicksPerMillisecond);

will still be higher than the int.MaxValue, that is the default, resulting in GetDueMiliseconds to return 0,

https://github.com/Azure/amqpnetlite/blob/3e42f0cc2e2981fd5c8376ef3338aa82b433eafe/src/Connection.cs#L962C1-L963C1

                    due = timeout > elapsed ? timeout - elapsed : 0;

which then closes the connection and throws the exception.

if (thisPtr.local > 0 &&

 if (thisPtr.local > 0 &&
                        GetDueMilliseconds(thisPtr.local, now, thisPtr.lastReceive) == 0)
                    {
                        thisPtr.connection.CloseInternal(
                            0,
                            new Error(ErrorCode.ConnectionForced)
                            {
                                Description = Fx.Format("Connection closed after idle timeout {0} ms", thisPtr.local)
                            });
                        thisPtr.SetTimerForClose();
                        return;
                    }

It would seem that the underlying issue is that AmqpSettings.IdleTimeout is an int but is used as uint.
Changing it to uint and initializing it with

                IdleTimeout = uint.MaxValue,

would fix this issue and would probably be more in line with how it is actually used.
Changing the IdleTimeout might result in a breaking change though.

@optimuserik
Copy link
Author

#577

@optimuserik
Copy link
Author

commit 54554af solves the issue.
Thank you.

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

No branches or pull requests

1 participant