classification
Title: Coercing strings and non-integer numbers to interpreter ID and channel ID
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eric.snow, mark.dickinson, miss-islington, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2019-09-02 09:11 by serhiy.storchaka, last changed 2019-09-25 15:56 by miss-islington. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 15652 merged serhiy.storchaka, 2019-09-02 17:06
PR 16129 closed miss-islington, 2019-09-13 19:50
PR 16145 merged serhiy.storchaka, 2019-09-14 16:09
PR 16227 merged serhiy.storchaka, 2019-09-17 10:42
PR 16394 merged miss-islington, 2019-09-25 15:36
Messages (12)
msg350972 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-09-02 09:11
Is it correct that strings, bytes objects, bytearrays, floats, fractions, decimals can be coerced to an interpreter ID?

>>> import _xxsubinterpreters as interpreters
>>> id = interpreters.InterpreterID('10', force=True)
>>> id == 10
True
>>> id == 10.1
True
>>> id == '1_0'
True
>>> id == bytearray(b'10')
True

Should not the constructor accept only ints and int-like objects and the comparison return True only for integer numbers?

There are other bugs here: raising OverflowError on comparison with large numbers and silencing all errors in some circumstances that can lead to returning a wrong result when errors like KeyboardInterrupt, RecursionError, MemoryError are occurred.
msg350984 - (view) Author: Mark Dickinson (mark.dickinson) * (Python committer) Date: 2019-09-02 11:10
Urk! Having `10 == id == 10.1` should imply `10 == 10.1`, by transitivity.

Are interpreter.InterpreterID objects hashable? If so, we've got a violation of the rule that `x == y` implies `hash(x) == hash(y)`.
msg351025 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-09-02 17:10
PR 15652 fixes some bugs. But I am not sure that floats and strings (and bytearrays, and fractions) should be accepted in constructors and several other functions. Although there was a test for strings. What is a use case? Should not an ID be an integer?
msg351274 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2019-09-07 00:00
Regardless of the intention of the _xx... author, intransitive equality is objectionable as it violates the assumption of sets and dicts.  We went through this before with decimal.Decimal when the original implementation had 1.0 == 1 == Decimal(1) but 1.0 != Decimal(1).  The latter was fixed.  Here, I think the fix should be to not make the id be equal to everything it can be derived from.
msg351322 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-09-08 10:28
I have doubts about accepting floats and bytes objects in all function that accept InterpreterID or ChannelID. Is it intentional? There are tests for strings.
msg351326 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-09-08 11:10
There is more serious issue: InterpreterID and ChannelID are declared as int subclasses, but actually they have incompatible structure, so using them as int causes a crash. For example, the following code is crashed:

    float(int(id)) == id
msg352390 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-09-13 19:50
New changeset bf169915ecdd42329726104278eb723a7dda2736 by Serhiy Storchaka in branch 'master':
bpo-38005: Fixed comparing and creating of InterpreterID and ChannelID. (GH-15652)
https://github.com/python/cpython/commit/bf169915ecdd42329726104278eb723a7dda2736
msg352442 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-09-14 16:36
New changeset f37a9831027ecfe948697cdb5e35b417805d94e5 by Serhiy Storchaka in branch '3.8':
[3.8] bpo-38005: Fixed comparing and creating of InterpreterID and ChannelID. (GH-15652) (GH-16145)
https://github.com/python/cpython/commit/f37a9831027ecfe948697cdb5e35b417805d94e5
msg352443 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-09-14 16:40
I would remove also converting a string to InterpreterID. The code would be simpler without it. I left it because there was explicit tests for it, but what is a use case for InterpreterID(string)?
msg352575 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2019-09-16 19:45
Yeah, dropping str support is fine.  It wouldn't be hard to add it back in if later we find it's useful. :)
msg353215 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2019-09-25 15:36
New changeset 543a3951a1c96bae0ea839eacec71d3b1a563a10 by Serhiy Storchaka in branch 'master':
bpo-38005: Remove support of string argument in InterpreterID(). (GH-16227)
https://github.com/python/cpython/commit/543a3951a1c96bae0ea839eacec71d3b1a563a10
msg353218 - (view) Author: miss-islington (miss-islington) Date: 2019-09-25 15:56
New changeset ca14f047b068cd0fb81ef3d5e7bea19fe8af1b58 by Miss Islington (bot) in branch '3.8':
bpo-38005: Remove support of string argument in InterpreterID(). (GH-16227)
https://github.com/python/cpython/commit/ca14f047b068cd0fb81ef3d5e7bea19fe8af1b58
History
Date User Action Args
2019-09-25 15:56:08miss-islingtonsetnosy: + miss-islington
messages: + msg353218
2019-09-25 15:36:08miss-islingtonsetpull_requests: + pull_request15976
2019-09-25 15:36:00serhiy.storchakasetmessages: + msg353215
2019-09-17 10:42:59serhiy.storchakasetpull_requests: + pull_request15825
2019-09-16 19:45:58eric.snowsetmessages: + msg352575
2019-09-14 16:40:54serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg352443

stage: patch review -> resolved
2019-09-14 16:36:22serhiy.storchakasetmessages: + msg352442
2019-09-14 16:09:32serhiy.storchakasetpull_requests: + pull_request15755
2019-09-13 19:50:38miss-islingtonsetpull_requests: + pull_request15740
2019-09-13 19:50:32serhiy.storchakasetmessages: + msg352390
2019-09-08 11:10:15serhiy.storchakasettype: behavior -> crash
2019-09-08 11:10:08serhiy.storchakasetmessages: + msg351326
2019-09-08 10:28:04serhiy.storchakasetmessages: + msg351322
2019-09-07 00:00:04terry.reedysetnosy: + terry.reedy
messages: + msg351274
2019-09-02 17:10:16serhiy.storchakasettype: behavior
components: + Interpreter Core
versions: + Python 3.8, Python 3.9
2019-09-02 17:10:02serhiy.storchakasetmessages: + msg351025
2019-09-02 17:06:41serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request15319
2019-09-02 16:55:12serhiy.storchakasettitle: Coercing strings and non-integer numbers to interpreter ID -> Coercing strings and non-integer numbers to interpreter ID and channel ID
2019-09-02 11:10:15mark.dickinsonsetnosy: + mark.dickinson
messages: + msg350984
2019-09-02 09:11:14serhiy.storchakacreate