Skip to content

Fix clock sync problems #13

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

Merged
merged 1 commit into from
Jan 9, 2014
Merged

Fix clock sync problems #13

merged 1 commit into from
Jan 9, 2014

Conversation

krillr
Copy link
Contributor

@krillr krillr commented Jan 9, 2014

When clocks are out of sync between hosts, improper logouts can occur. This stops this by not logging out when last activity is in the future in comparison to the local clock.

…n situations where clocks are out of sync between machines
jpic added a commit that referenced this pull request Jan 9, 2014
Fix clock sync problems
@jpic jpic merged commit a554581 into yourlabs:master Jan 9, 2014
@jpic
Copy link
Member

jpic commented Jan 9, 2014

We'll need a test case though

@yscumc
Copy link

yscumc commented Jan 9, 2014

I think it would make more sense to compare timedelta directly with timedelta instead of converting the type first, due to how python handles it internally.

See:

>>> a = timedelta(0,0,-1)
>>> b = timedelta(0,0,1)
>>> a < b
True

>>> a.days < b.days
True
>>> a.seconds < b.seconds
False
>>> a.microseconds < b.microseconds
False

>>> a
datetime.timedelta(-1, 86399, 999999)
>>> b
datetime.timedelta(0, 0, 1)

This weird behavior is also why I added the client_idle_for < 0 check in 7024081 from PR #3

In the long run, I think instead of

if delta.seconds >= EXPIRE_AFTER

this should be better

if delta >= timedelta(seconds=EXPIRE_AFTER)

@krillr
Copy link
Contributor Author

krillr commented Jan 9, 2014

Ah, yes you're right -- this also takes care of the case where it's been 1 day and 1 second since last activity.

@yscumc
Copy link

yscumc commented Jan 9, 2014

Wow I didn't even think of that.

That explains the weird bug which I encounter sometimes -- when I suspend my laptop, and resume it a long time later, sometimes the session would still be alive. I didn't have time to track down this problem and couldn't reproduce it reliably so I never added an issue here. The root cause of this problem is probably this comparison!

Thanks for solving the mystery of this frustrating bug!

@krillr
Copy link
Contributor Author

krillr commented Jan 9, 2014

Pull request #14 contains a unit test for future-time last activities and a change to using timedelta comparison, to avoid the issue we just talked about. Tests should be running shortly :)

@krillr
Copy link
Contributor Author

krillr commented Jan 9, 2014

For some reason travis keeps failing on Django 1.5 on what appears to be an unrelated test. No idea why o.o

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.

3 participants