Coercing strings and non-integer numbers to interpreter ID and channel ID

[serhiy-storchaka]

BPO	38005
Nosy	@terryjreedy, @mdickinson, @ericsnowcurrently, @serhiy-storchaka, @miss-islington
PRs	bpo-38005: Fixed comparing and creating of InterpreterID and ChannelID. #15652 [3.8] bpo-38005: Fixed comparing and creating of InterpreterID and ChannelID. (GH-15652) #16129 [3.8] bpo-38005: Fixed comparing and creating of InterpreterID and ChannelID. (GH-15652) #16145 bpo-38005: Remove support of string argument in InterpreterID(). #16227 [3.8] bpo-38005: Remove support of string argument in InterpreterID(). (GH-16227) #16394

^{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 2019-09-14.16:40:54.346>
created_at = <Date 2019-09-02.09:11:14.881>
labels = ['interpreter-core', '3.8', '3.9', 'type-crash']
title = 'Coercing strings and non-integer numbers to interpreter ID and channel ID'
updated_at = <Date 2019-09-25.15:56:08.382>
user = 'https://github.com/serhiy-storchaka'

bugs.python.org fields:

activity = <Date 2019-09-25.15:56:08.382>
actor = 'miss-islington'
assignee = 'none'
closed = True
closed_date = <Date 2019-09-14.16:40:54.346>
closer = 'serhiy.storchaka'
components = ['Interpreter Core']
creation = <Date 2019-09-02.09:11:14.881>
creator = 'serhiy.storchaka'
dependencies = []
files = []
hgrepos = []
issue_num = 38005
keywords = ['patch']
message_count = 12.0
messages = ['350972', '350984', '351025', '351274', '351322', '351326', '352390', '352442', '352443', '352575', '353215', '353218']
nosy_count = 5.0
nosy_names = ['terry.reedy', 'mark.dickinson', 'eric.snow', 'serhiy.storchaka', 'miss-islington']
pr_nums = ['15652', '16129', '16145', '16227', '16394']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue38005'
versions = ['Python 3.8', 'Python 3.9']

[serhiy-storchaka]

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.

[mdickinson]

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).

[serhiy-storchaka]

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?

[terryjreedy]

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.

[serhiy-storchaka]

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.

[serhiy-storchaka]

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

[serhiy-storchaka]

New changeset bf16991 by Serhiy Storchaka in branch 'master':
bpo-38005: Fixed comparing and creating of InterpreterID and ChannelID. (GH-15652)
bf16991

[serhiy-storchaka]

New changeset f37a983 by Serhiy Storchaka in branch '3.8':
[3.8] bpo-38005: Fixed comparing and creating of InterpreterID and ChannelID. (GH-15652) (GH-16145)
f37a983

[serhiy-storchaka]

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)?

[ericsnowcurrently]

Yeah, dropping str support is fine. It wouldn't be hard to add it back in if later we find it's useful. :)

[serhiy-storchaka]

New changeset 543a395 by Serhiy Storchaka in branch 'master':
bpo-38005: Remove support of string argument in InterpreterID(). (GH-16227)
543a395

[miss-islington]

New changeset ca14f04 by Miss Islington (bot) in branch '3.8':
bpo-38005: Remove support of string argument in InterpreterID(). (GH-16227)
ca14f04