#95 Class cache fails using servlet inheritance

closed-fixed
nobody
WebKit (58)
5
2010-10-28
2010-06-15
Patrick Gendron
No

The following code will fail:

Root.py:
from WebKit.Page import Page
class Root (Page):
def writeHTML (self):
Page.writeHTML (self)

A.py:
from Root import Root
class A (Root):
def writeHTML (self):
#assert (isinstance(self,Root))
Root.writeHTML (self)

B.py:
from A import A
class B (A):
def writeHTML (self):
A.writeHTML (self)

when successively calling servlets B -> Root -> A -> B after a fresh AppServer restart:

TypeError: unbound method writeHTML() must be called with Root instance as first argument (got B instance instead)

This seems to be caused by parents of B not being inserted into the cache on the first call to this servlet. Inspection of objects reveal that servlet A exists under two different ids during these calls. Indeed, on the second call to B, isinstance(self, Root) returns False in the writeHTML of class A.

Turning off the class cache bypasses this problem.

Can the cache mechanism be fixed to account for this particular situation?

Discussion

  • Hi Patrick, thanks for the detailed bug reported that helped me to reproduce the problem.

    I believe nobody has noticed this so far since usually you won't use a servlet class as both a base servlet class and a concrete servlet as you're doing here.

    You're right; the problem is caused by class A existing twice. However, the class cache mechanism is not really the culprit here; the problem just shows up earlier with the class cache enabled. But you can also reproduce it with a disabled class cache, e.g. by adding another servlet C inheriting from B and then calling C -> Root -> A -> C.

    The real cause of the problem is that A and B are forcefully reloaded by ServletFactory.importAsPackage() without also reloading their dependencies. So B inherits still from the old A which is not a subclass of Root anymore since Root has also been reloaded. To make everything consistent, if we forcefully reload a module, we must also reload its dependencies, otherwise we can get effects as the one you're reporting.

    Of course, we could simply not forcefully reload modules any more in ServletFactory.importAsPackage(), by setting forceReload=False in the last call inside the method. However, there is a good reason why forceReload has been set to True: By doing so, changes in servlets are picked up immediately without restarting the appserver, which is very convenient during development.

    I'm currently seeing only two solutions:

    1) Add an configuration option so that servlets are not forcefully reloaded, but the appserver reload mechanism (AutoReload=True in AppServer.config) is used instead. This will also work in situations like the one described by you, but can slow down development a little bit because the whole appserver needs to be restarted on every change to a servlet. Normally this happens only for changes in base classes and lib modules etc. which are less frequent.
    2) Forcefully reload all dependent modules when forcefuly reloading a servlet. But I fear finding the dependencies may be difficult and also slow.

    Do you agree with my analysis? Any suggestions?

     
  • Another improvement would be to do the forced reload only in the case that the module (the servlet file) changed. That way the described problem cannot happen in a production environment where the servlet files are not touched.

     
  • Your analysis looks good. So does your suggestions. Im my situation where I use base servlet classes and concrete base classes, there are situations where I have to manually restart the AppServer anyway since not all classes get automatically reloaded. I guess that everyone should be happy with using an option to not forcefully reload classes like in your solution 1.

    In any case, thanks for your help!

     
  • Ok, I think I'll change the forced reloading to happen only if 1) the file really has changed and 2) it has been activated with a new config setting (set by default).

     
  • Introduced the new server setting ReloadServletClasses in the trunk in r8141 (will be released as Webware 1.1).

    Patrick, can you check if this works for you? Anyway, I think you should refactor your modules so that your base servlet classes are never used as concrete servlets. Also, things work better if you do not use relative imports, i.e. if you write
    from WebKit.Examples.Root import Root
    instead
    from Root import Root
    I guess with the relative import, Python cannot figure out that the modules are the same, so tries to reload them even if they did not change.

     
    • status: open --> closed-fixed