Torque
  1. Torque
  2. TORQUE-68

DatabaseMap remains empty if Torque is stopped and restarted, causes NPEs

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 3.3-RC2
    • Fix Version/s: 3.3-RC3
    • Component/s: Runtime
    • Labels:
      None
    • Environment:
      Avalon, JUnit-Tests

      Description

      When running tests with the Fulcrum-Testcontainer, the TorqueComponent is instantiated on every lookup (setUp()) and shutdown on every tearDown(). In this scenario the first test succeeds and all others fail because the DatabaseMap contains no tables. This is probably caused by invalid static references to the DatabaseMap of the previous instance. This situation causes NullPointerExceptions in several BasePeer methods(e.g. processTables()).

        Activity

        Hide
        Thomas Fox added a comment -

        The title of this issue is misleading. Not the torque shutdown and reinitialisation is the problem, but the reinstantiation of the avalon component.
        This said, the problem is probably the constructor in the TorqueComponent clas. It just replaces the TorqueInstance in the torque class with itself, but it should also copy the Database maps (and maybe other stuff ?) to itself before discarding the old instance.

        Show
        Thomas Fox added a comment - The title of this issue is misleading. Not the torque shutdown and reinitialisation is the problem, but the reinstantiation of the avalon component. This said, the problem is probably the constructor in the TorqueComponent clas. It just replaces the TorqueInstance in the torque class with itself, but it should also copy the Database maps (and maybe other stuff ?) to itself before discarding the old instance.
        Hide
        Thomas Vandahl added a comment -

        I strongly believe that the reinstantiation of the component should work without any copying. If I start a new instance of Torque, no matter if in Avalon mode or not, I would expect everything to be as clean as if I started the first instance. This, in turn, means that we need to remove all static references in static classes to runtime-dependent objects.

        Show
        Thomas Vandahl added a comment - I strongly believe that the reinstantiation of the component should work without any copying. If I start a new instance of Torque, no matter if in Avalon mode or not, I would expect everything to be as clean as if I started the first instance. This, in turn, means that we need to remove all static references in static classes to runtime-dependent objects.
        Hide
        Thomas Fox added a comment -

        In principle I agree. But you are opening a big box here. If you remove all static references to runtime-dependent objects, I can see two options:
        1) move all static fields to an application-wide context, or
        2) do not use any central information any more.
        I am not sure if 2) is feasible without rewriting all of Torque. 1) is also difficult because you have to replace the self-registering mechanism of the peer classes when they are loaded (they register themselves in a central database map). But how should the peers register zhen ? The peer classes do not know which is the current context (there may be several of them), and the context does not know which peers are around (because they are classes provided by the user, and the context is compiled without these).
        The only sensible way around this is the mechanism which Greg introduced with the DatabaseMap.initialize() method, which uses a generated map of all database peer classes. The main argument against that approach was startup-time (if you have some hundred tables defined, and only need some of them, all the unused peer and map builder classes must be loaded); but maybe this can be improved if all the map builders are generated within one initializing class (or maybe one per database).

        But this is nothing which can be addressed in the 3.3 release cycle in my opinion.

        Show
        Thomas Fox added a comment - In principle I agree. But you are opening a big box here. If you remove all static references to runtime-dependent objects, I can see two options: 1) move all static fields to an application-wide context, or 2) do not use any central information any more. I am not sure if 2) is feasible without rewriting all of Torque. 1) is also difficult because you have to replace the self-registering mechanism of the peer classes when they are loaded (they register themselves in a central database map). But how should the peers register zhen ? The peer classes do not know which is the current context (there may be several of them), and the context does not know which peers are around (because they are classes provided by the user, and the context is compiled without these). The only sensible way around this is the mechanism which Greg introduced with the DatabaseMap.initialize() method, which uses a generated map of all database peer classes. The main argument against that approach was startup-time (if you have some hundred tables defined, and only need some of them, all the unused peer and map builder classes must be loaded); but maybe this can be improved if all the map builders are generated within one initializing class (or maybe one per database). But this is nothing which can be addressed in the 3.3 release cycle in my opinion.
        Hide
        Thomas Vandahl added a comment -

        I actually am in favour for 2), but let's talk about this in the 4.0 development. For 3.3, the problem might not be as bad as it sounds. My further investigations show that the problem buried here has been addressed by some measures but is not completely resolved. So what happens is this:

        When Torque is shutdown and restarted, a new instance is created. There is some code which tries to deal with unserialized om objects, but in our case, the static constructor of the peer class is not executed. So the Mapbuilder of the class is not registered with Torque and cannot be put into the BasePeer mapBuilders Hashtable. However, the mapBuilders Hashtable inside BasePeer is static itself and contains old and invalid entries from the previous run. It might be sufficient to clear this Hashtable during shutdown(). I'll check that out.

        Show
        Thomas Vandahl added a comment - I actually am in favour for 2), but let's talk about this in the 4.0 development. For 3.3, the problem might not be as bad as it sounds. My further investigations show that the problem buried here has been addressed by some measures but is not completely resolved. So what happens is this: When Torque is shutdown and restarted, a new instance is created. There is some code which tries to deal with unserialized om objects, but in our case, the static constructor of the peer class is not executed. So the Mapbuilder of the class is not registered with Torque and cannot be put into the BasePeer mapBuilders Hashtable. However, the mapBuilders Hashtable inside BasePeer is static itself and contains old and invalid entries from the previous run. It might be sufficient to clear this Hashtable during shutdown(). I'll check that out.
        Hide
        Thomas Vandahl added a comment -

        Allow repeated lookups to the Torque Avalon Component. The component now registers the existing MapBuilders with the new instance so that they are re-built during initialization. This is considered a temporary hack until the static constructors of the generated Peer classes go away.

        Show
        Thomas Vandahl added a comment - Allow repeated lookups to the Torque Avalon Component. The component now registers the existing MapBuilders with the new instance so that they are re-built during initialization. This is considered a temporary hack until the static constructors of the generated Peer classes go away.
        Hide
        Thomas Vandahl added a comment -

        The solution to this issue broke the handling of de-serialized objects for example in the Tomcat environment. Must copy the registered MapBuilders unconditionally. Need a testcase for this urgently.

        Show
        Thomas Vandahl added a comment - The solution to this issue broke the handling of de-serialized objects for example in the Tomcat environment. Must copy the registered MapBuilders unconditionally. Need a testcase for this urgently.
        Hide
        Thomas Vandahl added a comment -

        Problem still not fixed.

        Show
        Thomas Vandahl added a comment - Problem still not fixed.
        Hide
        Thomas Vandahl added a comment -

        Hopefully last attempt to fix this. A test case has been created which fails before and passes after the fix. What more can you ask...

        Show
        Thomas Vandahl added a comment - Hopefully last attempt to fix this. A test case has been created which fails before and passes after the fix. What more can you ask...

          People

          • Assignee:
            Thomas Vandahl
            Reporter:
            Thomas Vandahl
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development