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

[Windows] datetime.fromtimestamp(t) when 0 <= t <= 86399 fails on Python 3.6 #73283

Closed
pekkaklarck mannequin opened this issue Dec 28, 2016 · 16 comments
Closed

[Windows] datetime.fromtimestamp(t) when 0 <= t <= 86399 fails on Python 3.6 #73283

pekkaklarck mannequin opened this issue Dec 28, 2016 · 16 comments
Labels
3.7 (EOL) end of life 3.8 only security fixes OS-windows type-bug An unexpected behavior, bug, or error

Comments

@pekkaklarck
Copy link
Mannequin

pekkaklarck mannequin commented Dec 28, 2016

BPO 29097
Nosy @tim-one, @pfmoore, @abalkin, @vstinner, @tjguk, @bitdancer, @Preston-Landers, @jleclanche, @zware, @zooba, @ammaraskar, @pganssle
PRs
  • bpo-29097: Forego fold detection on windows for low timestamp values #2385
  • [3.7] bpo-29097: Forego fold detection on windows for low timestamp values (GH-2385) #8466
  • [3.6] bpo-29097: Forego fold detection on windows for low timestamp values (GH-2385) #8498
  • Files
  • forego_fold_detection.patch
  • truncate_negatives.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2018-07-27.16:33:03.375>
    created_at = <Date 2016-12-28.20:49:20.074>
    labels = ['3.8', 'type-bug', '3.7', 'OS-windows']
    title = '[Windows] datetime.fromtimestamp(t) when 0 <= t <= 86399 fails on Python 3.6'
    updated_at = <Date 2018-07-27.16:33:03.374>
    user = 'https://bugs.python.org/pekkaklarck'

    bugs.python.org fields:

    activity = <Date 2018-07-27.16:33:03.374>
    actor = 'ammar2'
    assignee = 'none'
    closed = True
    closed_date = <Date 2018-07-27.16:33:03.375>
    closer = 'ammar2'
    components = ['Windows']
    creation = <Date 2016-12-28.20:49:20.074>
    creator = 'pekka.klarck'
    dependencies = []
    files = ['46091', '46092']
    hgrepos = []
    issue_num = 29097
    keywords = ['patch']
    message_count = 16.0
    messages = ['284199', '284250', '284264', '284265', '284269', '284272', '284281', '284286', '284326', '296214', '296759', '296795', '309084', '322370', '322392', '322496']
    nosy_count = 14.0
    nosy_names = ['tim.peters', 'paul.moore', 'belopolsky', 'vstinner', 'tim.golden', 'r.david.murray', 'SilentGhost', 'planders', 'pekka.klarck', 'jleclanche', 'zach.ware', 'steve.dower', 'ammar2', 'p-ganssle']
    pr_nums = ['2385', '8466', '8498']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue29097'
    versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

    @pekkaklarck
    Copy link
    Mannequin Author

    pekkaklarck mannequin commented Dec 28, 2016

    For example:

    E:\>py -3.6 -c "import datetime; datetime.datetime.fromtimestamp(42)"
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    OSError: [Errno 22] Invalid argument

    Works fine at least with Python 2.6-2.7 and 3.3-3.5. Only tested on Windows (missing deadsnakes for easy Linux testing).

    I was also surprised to get OSError in general, but apparently fromtimestamp uses that instead of ValueError nowadays in some error situations.

    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Dec 29, 2016

    This doesn't seem likely that it's anything to do with datetime, could you try reproducing it in repl?

    @ammaraskar
    Copy link
    Member

    Can recreate successfully on windows, but not on linux:

    > C:\Python36\python.exe -c "import datetime; datetime.datetime.fromtimestamp(42)"
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
    OSError: [Errno 22] Invalid argument
    > C:\Python36\python.exe
    Python 3.6.0 (v3.6.0:41df79263a11, Dec 23 2016, 08:06:12) [MSC v.1900 64 bit (AMD64)] on win32
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import datetime; datetime.datetime.fromtimestamp(42)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    OSError: [Errno 22] Invalid argument

    @SilentGhost SilentGhost mannequin added OS-windows type-bug An unexpected behavior, bug, or error labels Dec 29, 2016
    @SilentGhost
    Copy link
    Mannequin

    SilentGhost mannequin commented Dec 29, 2016

    Cannot reproduce this with the tip on linux

    @bitdancer
    Copy link
    Member

    Sounds like it really is an OSError (that is, that the Windows OS is the source of the error). Whether or not we can or should do something about that is a separate question, though.

    @ammaraskar
    Copy link
    Member

    The 86,399 upperbound comes from this line (max_fold_seconds=86400):

    https://github.com/python/cpython/blob/master/Modules/_datetimemodule.c#L4277-L4278

    The bug is introduced as part of the fold detection in this commit: 56376ca

    Window's locatime_s function gives an EINVAL error when time is less than 0. Which is where the OSError comes from.

    https://msdn.microsoft.com/en-us/library/a442x3ye.aspx

    @ammaraskar
    Copy link
    Member

    I just ran the following script to check if there are any folds from timestamps [0, 86399] in any timezone.

      import datetime
      import pytz
    
      for tz in pytz.all_timezones:
          tz = pytz.timezone(tz)
          for i in range(86400):
              if datetime.datetime.fromtimestamp(i, tz).fold == 1:
                  print(str(tz)) 

    and it turns out there aren't any. I highly doubt any timezone is going to retroactively enable/disable DST during the epoch, so a potentially hacky fix is to just have a windows specific check for this value range.

    I'm adding the people involved in PEP-495 to the nosy list so they can give their input.

    @abalkin
    Copy link
    Member

    abalkin commented Dec 29, 2016

    I think we should just clip the negative lower probe value to 0 on Windows before passing it to local(). Also, if we still care about platforms with 32-bit time_t, we should check for over/under flow *before* calling local().

    @ammaraskar
    Copy link
    Member

    I've attached two patches that fix this behavior, one by simply foregoing the fold detection for this time range on windows (the patch that I'd argue is simpler and more readable)

    And one that truncates the passed values to local to not be negative. This one would have been simple but unfortunately there's the complication that there are two local calls and additionally, the second one cannot be truncated to exactly 0 because then we incorrectly detect a fold for the timestamp 0.

    @abalkin
    Copy link
    Member

    abalkin commented Jun 16, 2017

    I just ran the following script to check if there are any folds from timestamps [0, 86399] in any timezone.

    Ammar, I am not sure pytz timezones support PEP-495 conventions. Can you repeat your experiment using the latest dateutil.tz or test.datetimetester.ZoneInfo from Python 3.6 test suit?

    See <http://dateutil.readthedocs.io/en/stable/tz.html\>.

    @ammaraskar
    Copy link
    Member

    Hey, sorry for the late response. I just ran:

      import datetime
      from dateutil.zoneinfo import get_zonefile_instance
      import dateutil.tz
    
      zonenames = list(get_zonefile_instance().zones)
    
      for tz in zonenames:
          tz = dateutil.tz.gettz(tz)
          for i in range(86400):
              if datetime.datetime.fromtimestamp(i, tz).fold == 1:
                  print(str(tz))

    tz uses your OS's zone info so also posting my distro version:
    Debian 8.8 - 3.16.0-4-amd64 #1 SMP Debian 3.16.43-2 (2017-04-30) x86_64 GNU/Linux

    And I got the same result, no timezone with folds in these range.

    @ammaraskar
    Copy link
    Member

    Created a github pull request for the recommended patch along with a test: #2385

    @jleclanche
    Copy link
    Mannequin

    jleclanche mannequin commented Dec 27, 2017

    Anything holding up the PR? Looks good at a glance, would be nice to get this landed in time for 3.7.

    @eryksun eryksun added 3.7 (EOL) end of life 3.8 only security fixes labels Jun 28, 2018
    @vstinner vstinner changed the title datetime.fromtimestamp(t) when 0 <= t <= 86399 fails on Python 3.6 [Windows] datetime.fromtimestamp(t) when 0 <= t <= 86399 fails on Python 3.6 Jun 28, 2018
    @abalkin
    Copy link
    Member

    abalkin commented Jul 25, 2018

    New changeset 96d1e69 by Alexander Belopolsky (Ammar Askar) in branch 'master':
    bpo-29097: Forego fold detection on windows for low timestamp values (GH-2385)
    96d1e69

    @abalkin
    Copy link
    Member

    abalkin commented Jul 25, 2018

    New changeset 9736493 by Alexander Belopolsky (Miss Islington (bot)) in branch '3.7':
    bpo-29097: Forego fold detection on windows for low timestamp values (GH-2385) (GH-8466)
    9736493

    @abalkin
    Copy link
    Member

    abalkin commented Jul 27, 2018

    New changeset 6ea8a3a by Alexander Belopolsky (Ammar Askar) in branch '3.6':
    [3.6] bpo-29097: Forego fold detection on windows for low timestamp values (GH-2385) (GH-8498)
    6ea8a3a

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes OS-windows type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants