classification
Title: Bad multi-inheritance support in some libs like threading or multiprocessing
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 2.7
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: daniel.urban, eric.araujo, pitrou, rhettinger, sbt, serhiy.storchaka, thomas.chiroux, vstinner
Priority: normal Keywords: patch

Created on 2012-11-28 21:35 by thomas.chiroux, last changed 2017-04-09 06:05 by rhettinger. This issue is now closed.

Files
File name Uploaded Description Edit
diff_threading.py_2.7.patch thomas.chiroux, 2012-11-28 21:35 path for threading.Thread, python2.7
diff_unified_threading.py_2.7.patch thomas.chiroux, 2012-11-28 22:16
Messages (11)
msg176576 - (view) Author: Thomas Chiroux (thomas.chiroux) Date: 2012-11-28 21:35
when using multi-inheritance and super() in __init__(), super() tries to run each constructor once.
For this to work correctly, it is needed to call super() in each constructor, including for Classes directly inherited from object.

The sample below does not execute correctly because threading.Thread constructor does not call super()
(and because Thread inherit from _Verbose, _Verbose should also call super() )

Sample:
from threading import Thread


class Foo(object):
    def __init__(self):
        super(Foo, self).__init__()
        print('Foo constructor')
        self.param1 = 'foo param1'


class Bar(Thread, Foo):
    def __init__(self):
        super(Bar, self).__init__()
        print('Bar constructor')
        self.another_param1 = "bar another_param1"

    def run(self):
        print("Running (%s - %s)" % (self.another_param1, self.param1))


if __name__ == '__main__':
    threads = []
    for i in range(1, 10):
        thread = Bar()
        threads.append(thread)
        thread.start()

    for thread in threads:
        thread.join()


This sample work by simply inverting Thread and Foo :

(...)
class Bar(Foo, Thread):
(...)

The sample is about threading.Thread, but it's also the same for multiprocessing.Process, and maybe for other modules in stdlib.

attached is a proposed path for threading.Tread in 2.7

I've tested the issue and have the same behavior in 2.7 and in 3.3
msg176580 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2012-11-28 22:06
Can you please provide a diff in unified format? I don't see any super() in Lib/threading.py.
msg176581 - (view) Author: Thomas Chiroux (thomas.chiroux) Date: 2012-11-28 22:16
updated diff file, unified format
msg176588 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-11-28 23:22
The patch is liable to break programs which explicitly call base constructors because the constructor will be called more than once.

It also assumes that the __init__() method of all base classes should be called with no arguments (other than self).  That is true in your example, but won't be in general.
msg176592 - (view) Author: Thomas Chiroux (thomas.chiroux) Date: 2012-11-29 00:15
That's right.
Lets consider this 'patch' was more for illustrating my example (like:  this kind of modification may work) than to add directly into python core module... (which i'm not capable of)

But I think the problem remains: do you agree that Classes should include a super() call in their __init__ ?
[btw indeed a super() call with kwargs: super(ClassName, self).__init__(**kwargs)]
msg176602 - (view) Author: Richard Oudkerk (sbt) * (Python committer) Date: 2012-11-29 01:20
> But I think the problem remains: do you agree that Classes should include 
> a super() call in their __init__ ?

No, I don't.

I think super() is an attractive nuisance which is difficult to use correctly in an __init__() method, except in the trivial case where you only have one (non-mixin) base class.

See https://fuhm.net/super-harmful/
msg176772 - (view) Author: √Čric Araujo (eric.araujo) * (Python committer) Date: 2012-12-02 07:14
Adding Raymond, who thinks super is super, to the nosy list.

http://rhettinger.wordpress.com/2011/05/26/super-considered-super/
msg208327 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2014-01-17 08:22
Raymond, what do you think?
msg290033 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-03-23 09:35
The _Verbose class and verbose arguments for threading classes and functions were removed in issue13550. Thus this is 2.7-only issue now.
msg291334 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-04-08 15:32
Multi-inheritance is tricky to get right, both for the class being extended and the class extending it.  The APIs mentioned here were never designed for multiple inheritance and aren't supposed to support it.

Your use case would probably be better served by using composition rather than inheritance.  Speaking personally, I almost never subclass Thread, instead I make it an attribute of whatever class I'm writing.
msg291351 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2017-04-09 06:05
I'm going to decline this suggestion.  Antoine is right that this is a can of worms best less closed.  Likewise, Richard pointed out that there is potential to break existing code.

FWIW, the existence of super() does not imply that all existing classes need to rewritten with super calls.   "Cooperative multiple inheritance" implies a certain degree of pre-planning for meaningful cooperation between the classes.  For generic threading that doesn't seem plausible.

Also note that the referenced super-considered-super article shows how to incorporate non-cooperative classes in the chain without requiring that the original class be modified.
History
Date User Action Args
2017-04-09 06:05:06rhettingersetstatus: open -> closed
resolution: rejected
messages: + msg291351

stage: patch review -> resolved
2017-04-08 15:32:38pitrousetnosy: + pitrou
messages: + msg291334
2017-04-08 12:25:40serhiy.storchakasetassignee: rhettinger
2017-03-23 09:35:39serhiy.storchakasetnosy: + vstinner

messages: + msg290033
versions: - Python 3.3, Python 3.4
2014-01-17 08:22:00serhiy.storchakasetmessages: + msg208327
2014-01-09 19:47:08serhiy.storchakasetstage: patch review
versions: + Python 3.4
2012-12-02 07:14:04eric.araujosetnosy: + rhettinger, eric.araujo
messages: + msg176772
2012-11-29 08:16:04daniel.urbansetnosy: + daniel.urban
2012-11-29 01:20:30sbtsetmessages: + msg176602
2012-11-29 00:15:32thomas.chirouxsetmessages: + msg176592
2012-11-28 23:22:03sbtsetnosy: + sbt
messages: + msg176588
2012-11-28 22:16:25thomas.chirouxsetfiles: + diff_unified_threading.py_2.7.patch

messages: + msg176581
2012-11-28 22:06:47serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg176580
2012-11-28 21:35:08thomas.chirouxcreate