Geronimo
  1. Geronimo
  2. GERONIMO-4549

JMS resource jndi entries are not removed after uninstalling the JMS connect adapter

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.1, 2.1.2, 2.1.3, 2.1.4, 2.2
    • Fix Version/s: 2.1.5, 2.2
    • Component/s: ActiveMQ
    • Security Level: public (Regular issues)
    • Labels:
      None

      Description

      Steps to reproduce this problem:
      1. login admin console
      2. Create a ActiveMQ resource connector with the wizard
      3. Deploy it and check it is in running state
      4. Click J2EE connector to uninstall it
      5. Check JNDI viewer, you will see the JNDI entry still there, even you've uninstalled it.

        Activity

        Hide
        Jack Cai added a comment -

        I took quite some time to look into this problem since it's marked as a blocker and we are approching 2.1.4 release. I've nailed the problem down to the xbean-naming code, but I'm not sure how to fix it. Would like to get other's advice. Below is the summary of my findings -

        1. The sympton is that when a connection factory and a queue are created and started, and then stopped/uninstalled, their JNDI entry will continue to exist in the global context unless the server is restarted.

        2. When stoping/uninstalling them, org.apache.xbean.naming.context.AbstractContext.removeDeepBinding() will eventually be called, which finds the last context whose size > 2 in the context chain, then tries to remove the entry from this context (say Context A). This results in calling org.apache.xbean.naming.context.WritableContext$NestedWritableContext.removeBinding(), which uses removeNotEmptyContext=false to do removal. Unfortunately the context (say Context B) being removed (in Context A) is always not empty, resulting a ContextNotEmptyException which is ignored in org.apache.geronimo.gjndi.KernelContextGBean.removeBinding().

        I don't have enough background to understand why Context A is chosen this way and why removeNotEmptyContext is set to false. Can anyone shed some light here?

        Show
        Jack Cai added a comment - I took quite some time to look into this problem since it's marked as a blocker and we are approching 2.1.4 release. I've nailed the problem down to the xbean-naming code, but I'm not sure how to fix it. Would like to get other's advice. Below is the summary of my findings - 1. The sympton is that when a connection factory and a queue are created and started, and then stopped/uninstalled, their JNDI entry will continue to exist in the global context unless the server is restarted. 2. When stoping/uninstalling them, org.apache.xbean.naming.context.AbstractContext.removeDeepBinding() will eventually be called, which finds the last context whose size > 2 in the context chain, then tries to remove the entry from this context (say Context A). This results in calling org.apache.xbean.naming.context.WritableContext$NestedWritableContext.removeBinding(), which uses removeNotEmptyContext=false to do removal. Unfortunately the context (say Context B) being removed (in Context A) is always not empty, resulting a ContextNotEmptyException which is ignored in org.apache.geronimo.gjndi.KernelContextGBean.removeBinding(). I don't have enough background to understand why Context A is chosen this way and why removeNotEmptyContext is set to false. Can anyone shed some light here?
        Hide
        Jack Cai added a comment - - edited

        So I spend more time to understand the code. Guess I now know the reason why Context A is chosen that way (to quickly remove a chain of subcontext which only contains the object to be removed). But I still don't get why removeNotEmptyContext is set to false in org.apache.xbean.naming.context.WritableContext$NestedWritableContext.removeBinding().

        I propose 2 fixes as -

        1. Update org.apache.xbean.naming.context.WritableContext$NestedWritableContext.removeBinding(), new code below (change false to removeNotEmptyContext)

                protected boolean removeBinding(String name, boolean removeNotEmptyContext) throws NamingException {
                    if (WritableContext.this.removeBinding(bindingsRef, name, removeNotEmptyContext)) {
                        return true;
                    }
                    return super.removeBinding(name, removeNotEmptyContext);
                }
        

        2. Update the org.apache.xbean.naming.context.AbstractContext.removeDeepBinding() to remove the chain of subcontexts one by one starting from the leaf subcontexts, so that each subcontext IS empty when being removed and can thus be removed successfully even if removeNotEmptyContext is set to false.

        Any comments? I'll submit the patch tomorrow. But I'd like to get some advice beforehands.

        Show
        Jack Cai added a comment - - edited So I spend more time to understand the code. Guess I now know the reason why Context A is chosen that way (to quickly remove a chain of subcontext which only contains the object to be removed). But I still don't get why removeNotEmptyContext is set to false in org.apache.xbean.naming.context.WritableContext$NestedWritableContext.removeBinding(). I propose 2 fixes as - 1. Update org.apache.xbean.naming.context.WritableContext$NestedWritableContext.removeBinding(), new code below (change false to removeNotEmptyContext) protected boolean removeBinding( String name, boolean removeNotEmptyContext) throws NamingException { if (WritableContext. this .removeBinding(bindingsRef, name, removeNotEmptyContext)) { return true ; } return super .removeBinding(name, removeNotEmptyContext); } 2. Update the org.apache.xbean.naming.context.AbstractContext.removeDeepBinding() to remove the chain of subcontexts one by one starting from the leaf subcontexts, so that each subcontext IS empty when being removed and can thus be removed successfully even if removeNotEmptyContext is set to false. Any comments? I'll submit the patch tomorrow. But I'd like to get some advice beforehands.
        Hide
        Joe Bohn added a comment -

        Perhaps I don't fully understand the problem ... but I attempted to recreate it and in my case the jndi entry was removed.

        I did the following:

        • selected Services->JMS Resources from the navigation
        • created a new resource group for ActiveMQ as follows:
        • specified a group of myRG and kept the defaults for all else
        • created a new connection factory under the Resource Group called myFactory and kept the defaults for all else
        • deployed the plan
        • validated that the resource group was running and visible in the JMS Resources view, the J2EE Connectors view (and running) and the JNDI viewer under ResourceAdapterModule
        • from the J2EE Connectors view I uninstalled my resource group rar
        • It was removed from the J2EE Connectors view, the JMS Resources view, and the JNDI Viewer

        Am I completely missing the issue?

        Show
        Joe Bohn added a comment - Perhaps I don't fully understand the problem ... but I attempted to recreate it and in my case the jndi entry was removed. I did the following: selected Services->JMS Resources from the navigation created a new resource group for ActiveMQ as follows: specified a group of myRG and kept the defaults for all else created a new connection factory under the Resource Group called myFactory and kept the defaults for all else deployed the plan validated that the resource group was running and visible in the JMS Resources view, the J2EE Connectors view (and running) and the JNDI viewer under ResourceAdapterModule from the J2EE Connectors view I uninstalled my resource group rar It was removed from the J2EE Connectors view, the JMS Resources view, and the JNDI Viewer Am I completely missing the issue?
        Hide
        David Jencks added a comment -

        Jack,

        I don't completely understand the xbean jndi code however I think the reason for some of the things like removeNotEmptyContext are to support federated jndi contexts. So I recommend you try to understand federated jndi contexts better than I do before changing the code... or come up with more unit tests.

        Show
        David Jencks added a comment - Jack, I don't completely understand the xbean jndi code however I think the reason for some of the things like removeNotEmptyContext are to support federated jndi contexts. So I recommend you try to understand federated jndi contexts better than I do before changing the code... or come up with more unit tests.
        Hide
        Jack Cai added a comment -

        Thanks Joe and David for your response!

        Joe, if you look at the "Global context" after uninstalling the connector, you will see the connection factory name is still there.

        I'll digg more into this.

        Show
        Jack Cai added a comment - Thanks Joe and David for your response! Joe, if you look at the "Global context" after uninstalling the connector, you will see the connection factory name is still there. I'll digg more into this.
        Hide
        Joe Bohn added a comment -

        I see no trace of the name anywhere after I uninstall - even under Global context. Are you sure you are seeing this problem with the latest 2.1.4-SNAPSHOT? Either it's been resolved or there is some nuance to the scenario that I'm not hitting (which might help narrow down the cause and proposed fix a bit more).

        Show
        Joe Bohn added a comment - I see no trace of the name anywhere after I uninstall - even under Global context. Are you sure you are seeing this problem with the latest 2.1.4-SNAPSHOT? Either it's been resolved or there is some nuance to the scenario that I'm not hitting (which might help narrow down the cause and proposed fix a bit more).
        Hide
        Joe Bohn added a comment -

        Ok, I see the failure now. You must add a destination in addition to a connection factory. Adding a connection factory alone to the resource group and all still works as expected on uninstall - the connection factory jndi name is removed from the global context. However, if you add both a connection factory and a destination into the same plan and deploy it both the connection factory and destination will remain under global context even after uninstall.

        Show
        Joe Bohn added a comment - Ok, I see the failure now. You must add a destination in addition to a connection factory. Adding a connection factory alone to the resource group and all still works as expected on uninstall - the connection factory jndi name is removed from the global context. However, if you add both a connection factory and a destination into the same plan and deploy it both the connection factory and destination will remain under global context even after uninstall.
        Hide
        Joe Bohn added a comment -

        I moved this from blocker back to minor. I saw no justification for why it was made a blocking issue. I also verified the same issue is present in 2.1.1, 2.1.2, and 2.1.3.

        Show
        Joe Bohn added a comment - I moved this from blocker back to minor. I saw no justification for why it was made a blocking issue. I also verified the same issue is present in 2.1.1, 2.1.2, and 2.1.3.
        Hide
        Joe Bohn added a comment -

        BTW, one other interesting tidbit. Once you have at least one jndi resource entry that is not removed subsequent entries will likewise not be removed (even if you then deploy and uninstall a resource group with just a connector).

        For example, if you deploy and undeploy a resource group with just one connector and no destination the connector entry will be removed from the jndi global context.
        If you deploy a resource group with both a connector and destination the global context entries will not be removed on uninstall.
        And, if you subsequently deploy and undeploy another resource group with just a connector (no destination) after the failure to cleanup mentioned above the new connector will also continue to exist in global context after the uninstall.

        Show
        Joe Bohn added a comment - BTW, one other interesting tidbit. Once you have at least one jndi resource entry that is not removed subsequent entries will likewise not be removed (even if you then deploy and uninstall a resource group with just a connector). For example, if you deploy and undeploy a resource group with just one connector and no destination the connector entry will be removed from the jndi global context. If you deploy a resource group with both a connector and destination the global context entries will not be removed on uninstall. And, if you subsequently deploy and undeploy another resource group with just a connector (no destination) after the failure to cleanup mentioned above the new connector will also continue to exist in global context after the uninstall.
        Hide
        Jack Cai added a comment -

        I've got some progress now.

        1. For the original problem, it happens in such scenario: if there are two names which share some common intermediate parts, e.g., "test/test1/GBean/R1" and "test/test2/GBean/R2", then any attempts to remove any of them will fail. This can be fixed by using passed-in removeNotEmptyContext instead of "false" in org.apache.xbean.naming.context.WritableContext$NestedWritableContext.removeBinding(). I've read through related code and feel it's a safe update.

        2. There is another more serious problem exposed after the above problem is fixed, i.e., the index entries in WritableContext will not be removed when the corresponding object is removed. This will result in subsequent "lookup" for that object continuing to succeed. I've also fixed this.

        I wrote two testcases for future regression. The patched code passes all previous unit tests in addition to the new ones. Also verified the JMS resource issue in Admin Console is now resolved.

        Show
        Jack Cai added a comment - I've got some progress now. 1. For the original problem, it happens in such scenario: if there are two names which share some common intermediate parts, e.g., "test/test1/GBean/R1" and "test/test2/GBean/R2", then any attempts to remove any of them will fail. This can be fixed by using passed-in removeNotEmptyContext instead of "false" in org.apache.xbean.naming.context.WritableContext$NestedWritableContext.removeBinding(). I've read through related code and feel it's a safe update. 2. There is another more serious problem exposed after the above problem is fixed, i.e., the index entries in WritableContext will not be removed when the corresponding object is removed. This will result in subsequent "lookup" for that object continuing to succeed. I've also fixed this. I wrote two testcases for future regression. The patched code passes all previous unit tests in addition to the new ones. Also verified the JMS resource issue in Admin Console is now resolved.
        Hide
        Donald Woods added a comment -

        Joe, +1 for getting this into 2.1.4, even if it delays a RC by a couple days....

        Show
        Donald Woods added a comment - Joe, +1 for getting this into 2.1.4, even if it delays a RC by a couple days....
        Hide
        Joe Bohn added a comment -

        So it seems that this really is an XBean issue. We should create a JIRA under their project: http://issues.apache.org/jira/browse/XBEAN
        For us to get this into Geronimo 2.1.4 we would need another xbean release first. It would most likely delay 2.1.4 more than just a few days.

        Show
        Joe Bohn added a comment - So it seems that this really is an XBean issue. We should create a JIRA under their project: http://issues.apache.org/jira/browse/XBEAN For us to get this into Geronimo 2.1.4 we would need another xbean release first. It would most likely delay 2.1.4 more than just a few days.
        Hide
        Jack Cai added a comment -

        If pulling in this fix solely delays Geronimo 2.1.4 release, then I'm inclined to exclude it for the time being. After all, it's almost 6 months now since last Geronimo release. Let's just get it into the trunk.

        Opened an XBean issue: https://issues.apache.org/jira/browse/XBEAN-122

        Show
        Jack Cai added a comment - If pulling in this fix solely delays Geronimo 2.1.4 release, then I'm inclined to exclude it for the time being. After all, it's almost 6 months now since last Geronimo release. Let's just get it into the trunk. Opened an XBean issue: https://issues.apache.org/jira/browse/XBEAN-122
        Hide
        Jarek Gawor added a comment -

        Updated affects/fix versions as this won't get fixed in time for 2.1.4.

        Show
        Jarek Gawor added a comment - Updated affects/fix versions as this won't get fixed in time for 2.1.4.
        Hide
        Jack Cai added a comment -

        Closing this issue as it's a XBean issue.

        Show
        Jack Cai added a comment - Closing this issue as it's a XBean issue.
        Hide
        Forrest Xia added a comment -

        Until we merge in the XBean fix to g, I don't think we can close this bug now.

        Leave it open, so that we can filter it out some day in future and verify it if XBean patch is merged.

        Show
        Forrest Xia added a comment - Until we merge in the XBean fix to g, I don't think we can close this bug now. Leave it open, so that we can filter it out some day in future and verify it if XBean patch is merged.
        Hide
        David Jencks added a comment -

        Assuming this is actually fixed in xbean... trunk has been using xbean snapshots for a long time. Rev 803714 makes the 2.1 branch use xbean snapshots. I didn't see any obvious problems.

        Show
        David Jencks added a comment - Assuming this is actually fixed in xbean... trunk has been using xbean snapshots for a long time. Rev 803714 makes the 2.1 branch use xbean snapshots. I didn't see any obvious problems.

          People

          • Assignee:
            Jack Cai
            Reporter:
            Forrest Xia
          • Votes:
            1 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development