python / cpython

The Python programming language
https://www.python.org
Other
63.45k stars 30.38k forks source link

SystemError: ../Objects/longobject.c:998: bad argument to internal function #68030

Closed doko42 closed 9 years ago

doko42 commented 9 years ago
BPO 23842
Nosy @warsaw, @doko42, @vstinner, @benjaminp, @serhiy-storchaka
Files
  • posix_dev_t_converter.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: ```python assignee = 'https://github.com/serhiy-storchaka' closed_at = created_at = labels = ['extension-modules', 'release-blocker'] title = 'SystemError: ../Objects/longobject.c:998: bad argument to internal function' updated_at = user = 'https://github.com/doko42' ``` bugs.python.org fields: ```python activity = actor = 'serhiy.storchaka' assignee = 'serhiy.storchaka' closed = True closed_date = closer = 'serhiy.storchaka' components = ['Extension Modules'] creation = creator = 'doko' dependencies = [] files = ['38790'] hgrepos = [] issue_num = 23842 keywords = ['patch'] message_count = 8.0 messages = ['239826', '239828', '239830', '239832', '239833', '239841', '239842', '241615'] nosy_count = 6.0 nosy_names = ['barry', 'doko', 'vstinner', 'benjamin.peterson', 'python-dev', 'serhiy.storchaka'] pr_nums = [] priority = 'release blocker' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue23842' versions = ['Python 2.7'] ```

    doko42 commented 9 years ago

    an unreleased version of cinder fails a test when run with python 2.7 from the branch. I don't have a test case yet, but looking at the error message and list of fixed issues after the 2.7.9 release, the fix for issue bpo-23098 could be the culprit.

    FAIL: cinder.tests.test_utils.GetBlkdevMajorMinorTestCase.test_get_blkdev_major_minor_file cinder.tests.test_utils.GetBlkdevMajorMinorTestCase.test_get_blkdev_major_minor_file ----------------------------------------------------------------------

    _StringException: Traceback (most recent call last):
      File "/build/buildd/cinder-2015.1~b3/cinder/tests/test_utils.py", line 721, in test_get_blkdev_major_minor_file
        dev = self._test_get_blkdev_major_minor_file('/dev/made_up_disk1')
      File "/usr/lib/python2.7/dist-packages/mock.py", line 1210, in patched
        return func(*args, **keywargs)
      File "/build/buildd/cinder-2015.1~b3/cinder/tests/test_utils.py", line 712, in _test_get_blkdev_major_minor_file
        dev = utils.get_blkdev_major_minor(test_file)
      File "/build/buildd/cinder-2015.1~b3/cinder/utils.py", line 656, in get_blkdev_major_minor
        return get_blkdev_major_minor(devpath, False)
      File "/build/buildd/cinder-2015.1~b3/cinder/utils.py", line 645, in get_blkdev_major_minor
        return '%d:%d' % (os.major(st.st_rdev), os.minor(st.st_rdev))
    SystemError: ../Objects/longobject.c:998: bad argument to internal function
    Traceback (most recent call last):
    _StringException: Empty attachments:
      stderr
      stdout
    vstinner commented 9 years ago

    return '%d:%d' % (os.major(st.st_rdev), os.minor(st.st_rdev))

    Can you check if the error comes from os.major() or os.minor(), and please provide the value of st_rdev?

    You can modify the unit test to catch SystemError and raise a new exception with more information.

    doko42 commented 9 years ago

    so the culprit is an "optimization":

    "Committed to 2.7 with small change: stat() and makedev() should return int instead of long if possible."

    the test case however expects a long. st_dev is now not always a long, but an int or long depending on the value.

    doko42 commented 9 years ago

    no, simpler:

    >>> import os
    >>> os.minor(os.makedev(8,65))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    SystemError: ../Objects/longobject.c:998: bad argument to internal function
    >>> os.major(os.makedev(8,65))
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    SystemError: ../Objects/longobject.c:998: bad argument to internal function
    doko42 commented 9 years ago

    casting to a long works ...

    >>> os.major(long(os.makedev(8,65)))
    8

    so maybe revert that optimization for the released branches?

    vstinner commented 9 years ago

    Obviously, an unit test is missing ;)

    serhiy-storchaka commented 9 years ago

    Oh, this is a catch when backport to 2.7. Here is a patch that makes dev_t converter to support int and int-like types. Added tests (surprisingly there were no tests for makedev/major/minor at all).

    so maybe revert that optimization for the released branches?

    It is not an optimization, but required for backward compatibility. Third-party code can expect int and only int.

    1762cc99-3127-4a62-9baf-30c3d0f51ef7 commented 9 years ago

    New changeset cd7d51b5c951 by Serhiy Storchaka in branch '2.7': Issue bpo-23842: os.major(), os.minor() and os.makedev() now support ints again. https://hg.python.org/cpython/rev/cd7d51b5c951

    New changeset 998d967b8a57 by Serhiy Storchaka in branch '3.4': Issue bpo-23842: Added tests for os.major(), os.minor() and os.makedev(). https://hg.python.org/cpython/rev/998d967b8a57

    New changeset d477da6f287f by Serhiy Storchaka in branch 'default': Issue bpo-23842: Added tests for os.major(), os.minor() and os.makedev(). https://hg.python.org/cpython/rev/d477da6f287f