HiveMind
  1. HiveMind
  2. HIVEMIND-116

add destroy-method attribute to the construct element of BuilderFactory

    Details

    • Type: New Feature New Feature
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.1
    • Fix Version/s: None
    • Component/s: framework
    • Labels:
      None

      Description

      In order to keep object agnostic of container, please add attributes to the BuilderFactory contruct element that support the standard lifecycle management of Hivemind. This request is for the destroy step in the life cycle of a service. This attribute could be used instead of RegistryShutdownListener and Discardable.

      The following is a specification should be a reasonable start.

      add a destroy-method attribute to the construct element recognized by BuilderFactory

      When the destroy-method is specified, the method must be a public void method taking no parameters. This method is called when the registry is shutdown for primitive, singleton, and pooled services. This method is
      called when the thread is cleaned up for a threaded service. When autowire-service is enabled and destroy-method is not specified, BuilderFactory will introspect for a method called destroyService() and
      treat it as a destroy-method. In the case of singleton and pooled services, if the service implements the RegistryShutdownListener, the destroy-method is called before the RegistryShutdownListener.registryDidShutdown() method. In the case of threaded services, if the service implements the Discardable interface, the destroy-method is called before the
      Discardable.threadDidDiscardService() method.

        Activity

        Hide
        Johan Lindquist added a comment -

        Please find the patch attached providing one of the possible solutions to this.

        Note that due to the nature of this enhancement, it also fixes HIVEMIND-117

        Comments are appreciated.

        Show
        Johan Lindquist added a comment - Please find the patch attached providing one of the possible solutions to this. Note that due to the nature of this enhancement, it also fixes HIVEMIND-117 Comments are appreciated.
        Hide
        Johan Lindquist added a comment -

        Sure, will go with that.

        Thanks for the help.

        Show
        Johan Lindquist added a comment - Sure, will go with that. Thanks for the help.
        Hide
        James Carman added a comment -

        Can't you just log the error with the error handler? Then, the error handler implementation (whether default or strict) can decide what to do. This is how we handle other stuff like this I believe.

        Show
        James Carman added a comment - Can't you just log the error with the error handler? Then, the error handler implementation (whether default or strict) can decide what to do. This is how we handle other stuff like this I believe.
        Hide
        Johan Lindquist added a comment -

        Well, not necessarily - I can throw an exception, in which case the error handler will deal with it. But I can also log it (directly) as a warning.

        The latter option is required if you are processing a final class (or even an info message at that point) since there is no way you're gonna be able to decorate it at all (have already stumbled on it for some of Hivemind internal classes).

        Get's a little hairy, cause the case is as follows:

        1) The user specifies a destroyMethod, say "doShutdown"
        2) The class contains (already) a method registryDidShutdown, which is final
        3) The class needs to be enhanced (add the RegistryShutdownListener interface) as well as override the registryDidShutdown (in order to call the doShutdown method).

        That's when it blows up somewhat ...

        I guess an exception would be ok (rather than a warning) in this case ... Or what do you think?

        Show
        Johan Lindquist added a comment - Well, not necessarily - I can throw an exception, in which case the error handler will deal with it. But I can also log it (directly) as a warning. The latter option is required if you are processing a final class (or even an info message at that point) since there is no way you're gonna be able to decorate it at all (have already stumbled on it for some of Hivemind internal classes). Get's a little hairy, cause the case is as follows: 1) The user specifies a destroyMethod, say "doShutdown" 2) The class contains (already) a method registryDidShutdown, which is final 3) The class needs to be enhanced (add the RegistryShutdownListener interface) as well as override the registryDidShutdown (in order to call the doShutdown method). That's when it blows up somewhat ... I guess an exception would be ok (rather than a warning) in this case ... Or what do you think?
        Hide
        James Carman added a comment -

        I guess that would depend on how the ErrorHandler wants to handle it, correct?

        Show
        James Carman added a comment - I guess that would depend on how the ErrorHandler wants to handle it, correct?
        Hide
        Johan Lindquist added a comment -

        Another one for the group - when attempting to decorate the implementation class & the destroy method does exist but is final, javassist will throw an exception that the a class is being extended, overriding a final method. The check can easily be done, but the question is how to report it - warn the user (i.e log it) or throw an exception?

        Show
        Johan Lindquist added a comment - Another one for the group - when attempting to decorate the implementation class & the destroy method does exist but is final, javassist will throw an exception that the a class is being extended, overriding a final method. The check can easily be done, but the question is how to report it - warn the user (i.e log it) or throw an exception?
        Hide
        Johan Lindquist added a comment -

        By enhancing the core class, the implementation class will change name - I am currently using a Hivemind utility class to generate this. However, when debugging calls in the original implementation, the class name used is not very intuitive - I am thinking that the enhanced class should be called something like HMEnanced<OriginalClassName> or <OriginalClassName>$HMEnanced to not confuse users too much. Any particular thoughts or suggestions?

        Show
        Johan Lindquist added a comment - By enhancing the core class, the implementation class will change name - I am currently using a Hivemind utility class to generate this. However, when debugging calls in the original implementation, the class name used is not very intuitive - I am thinking that the enhanced class should be called something like HMEnanced<OriginalClassName> or <OriginalClassName>$HMEnanced to not confuse users too much. Any particular thoughts or suggestions?
        Hide
        Johan Lindquist added a comment -

        I have looked at the implementation of this and have approached it the following way:

        In the BuilderFactoryLogic (instantiateCoreServiceInstance() method), add a addShutdownHooks(implementationClass)

        The addShutdownHooks returns the (possibly) modified class instance, implementing the above.

        Looks good in practice - the service models work exactly the same (no change required).

        If this sounds like the correct place to add this functionality, I will can ahead and cleanup/add more tests to cover this.

        Show
        Johan Lindquist added a comment - I have looked at the implementation of this and have approached it the following way: In the BuilderFactoryLogic (instantiateCoreServiceInstance() method), add a addShutdownHooks(implementationClass) The addShutdownHooks returns the (possibly) modified class instance, implementing the above. Looks good in practice - the service models work exactly the same (no change required). If this sounds like the correct place to add this functionality, I will can ahead and cleanup/add more tests to cover this.
        Hide
        Richard Hensley added a comment -

        Knut,

        I've done some research on this issue and the following seems to be a reasonable design for this feature.

        I think the following would work for RegistryDidShutdown emulation of destroy-method.

        Validate that the name destroy-method exists on the service, use "destroyService" as a default when the destroy-method attribute is not supplied.

        If the destroy-method exists on the service, enhance the class in the following ways.

        • If the core service does not implement the RegistryDidShutdown interface, add it.
        • If the core service does not have a registryDidShutdown method, add it.
        • Add a call to the "destroy-method" before the rest of the code in the registryDidShutdown method.

        I think the following would work for Discardable emulation of destroy-method

        If the destroy-method exists on the service, enhance the class in the following ways.

        • If the core service does not implement the Discardable interface, add it.
        • If the core service does not have a threadDidDiscardService method, add it.
        • Add a call to the "destroy-method" before the rest of the code in the threadDidDiscardService method.

        Because the ThreadServiceModel documents that the RegistryShutdownListener is not used, and the Pooled and Singleton Service document that Discardable is not used, there should not be any double calling of the destroy method.

        What does everybody think?

        Show
        Richard Hensley added a comment - Knut, I've done some research on this issue and the following seems to be a reasonable design for this feature. I think the following would work for RegistryDidShutdown emulation of destroy-method. Validate that the name destroy-method exists on the service, use "destroyService" as a default when the destroy-method attribute is not supplied. If the destroy-method exists on the service, enhance the class in the following ways. If the core service does not implement the RegistryDidShutdown interface, add it. If the core service does not have a registryDidShutdown method, add it. Add a call to the "destroy-method" before the rest of the code in the registryDidShutdown method. I think the following would work for Discardable emulation of destroy-method If the destroy-method exists on the service, enhance the class in the following ways. If the core service does not implement the Discardable interface, add it. If the core service does not have a threadDidDiscardService method, add it. Add a call to the "destroy-method" before the rest of the code in the threadDidDiscardService method. Because the ThreadServiceModel documents that the RegistryShutdownListener is not used, and the Pooled and Singleton Service document that Discardable is not used, there should not be any double calling of the destroy method. What does everybody think?

          People

          • Assignee:
            Unassigned
            Reporter:
            Richard Hensley
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:

              Development