_register is not safe

[reowen]

BPO	1267540
Nosy	@loewis

^{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 = 'https://github.com/loewis'
closed_at = <Date 2005-08-24.19:30:46.000>
created_at = <Date 2005-08-23.23:29:05.000>
labels = ['invalid', 'expert-tkinter']
title = '_register is not safe'
updated_at = <Date 2005-08-24.19:30:46.000>
user = 'https://bugs.python.org/reowen'

bugs.python.org fields:

activity = <Date 2005-08-24.19:30:46.000>
actor = 'reowen'
assignee = 'loewis'
closed = True
closed_date = None
closer = None
components = ['Tkinter']
creation = <Date 2005-08-23.23:29:05.000>
creator = 'reowen'
dependencies = []
files = []
hgrepos = []
issue_num = 1267540
keywords = []
message_count = 3.0
messages = ['26103', '26104', '26105']
nosy_count = 2.0
nosy_names = ['loewis', 'reowen']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue1267540'
versions = ['Python 2.4']

[reowen]

The _register function is not safe. It uses the following code to
generate a unique string to identify a python callable entity:

 f = CallWrapper(func, subst, self).__call__
 name = repr(id(f))

It then adds the function name if func has a __name__ attribute, but
bound methods don't have that, and it is bound methods that cause
the problem because they don't have a constant id number unless
somebody bothers to keep an explicit reference to the bound
reference around (keeping a reference to the object itself isn't
enough).

For example:
class foo:
def meth1(self):
pass
def meth2(self):
pass

f = foo()
print id(f)
print id(f.meth1)
print id(f.meth2)

Odds are good that the last two printed numbers will be the same
even though they are for two different bound methods. The id of f
will persist and remain unique as long as the object exists, but the id
of its bound methods will not. Even though one can call the bound
methods as long as the object persists.

I don't know how to best handle this. Possibilities include:

• Use hash(func) instead of id(func).
  I don't know enough about the built in hash to know if that is safe.
  Certainly it is the right idea. One wants a function that has the same
  value if the callable entity is the same and a different value
  otherwise.

• Generate a new hash function that is sure to work.
  "paolino" suggested the following hash function for bound methods
  (when I first asked about this problem on comp.lang.python):
  id(boundMethod.im_self)^id(boundMethod.im_func) this is probably
  the simplest solution if hash itself isn't safe, but it requires a some
  fiddling to figure out what kind of callable entity one has, and thus
  what hash equation is suitable.

• Keep a reference to the callable entity, so its id number cannot be
  reused.
  That sounds hard to do safely; it would be all too easy to introduce
  a memory leak. It can probably be done fairly easily for the case
  cleanup=1, but there is code that uses cleanup=0 (including
  bind_all and bind_class).

-- Russell

[loewis]

Logged In: YES
user_id=21627

Can you provide a test case that demonstrates the problem?

I don't think there is anything wrong with that code. We
don't take the id of func, we take the id of f, where f is
CallWrapper(...).__call__. f gets then passed to
tk.createcommand, which in turn puts f into
PythonCmd_ClientData. So as long as the Tcl command lives, f
(which is indeed a method) is referenced, which in turn
means that no other method can have the same id.

You are also wrong that the name is not added for bound
methods. It looks at im_func, and then takes the name of the
function in the bound method.

>>> f.meth1.im_func.__name__
'meth1'

[reowen]

Logged In: YES
user_id=431773

You are right. I should have realized that cratecommand kept a reference
to the function (else how could it call it?).

I replicated _register so as to not use a Tkinter internal function, and at
some point ended up taking the id of the wrong thing. Sorry for the false
alarm.