[signals] Storing id()s instead of actual instances considered harmful
|Reported by:||Owned by:||nobody|
|Has patch:||no||Needs documentation:||no|
|Needs tests:||no||Patch needs improvement:||no|
The signal dispatcher module goes to great lengths to avoid keeping references to the actual senders and receivers and instead holds ids and weakrefs. Aside from the fact that the whole idea is debatable at least (never seen a use case that relied on implicit garbage collection of senders/receivers without doing an explicit disconnect()), it is particularly error prone if one happens to send() an equal but not identical sender to the one used in connect().
I was bitten by this recently in an app that uses plain byte strings as senders. Then some other app called send() passing the sender string as unicode (because it was resolved from a template), which led to an obscure bug since the receivers were not called. And in case someone comes back with "well duh, it's your responsibility to ensure you send byte strings, not the framework's", it gets worse: simply sending str(sender) still fails because it creates a *new* string instance, which has different id() from the one connected. What eventually worked was intern(str(sender)) but that's hardly acceptable. Not only it is an ugly kludge but it works only for byte strings; unicode, integers, tuples, etc. are still bad choices for senders.
The ideal solution would be to get rid of id()s altogether and store the actual senders. If for some reason this is not possible or desirable, a new 'sender_dispatcher_uid' (in the same vein as 'dispatcher_uid') optional parameter should be introduced.
Change History (8)
comment:1 Changed 7 years ago by
|Patch needs improvement:||unset|
|Status:||new → closed|
comment:3 Changed 7 years ago by
|Status:||reopened → closed|
|Triage Stage:||Unreviewed → Accepted|