[jira] [Commented] (FELIX-5274) remove callback fails after manually removing dynamic dependencies

["Pierre De Rop (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FELIX-5274?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15519645#comment-15519645 ] 

Pierre De Rop commented on FELIX-5274:
--------------------------------------

Hi Bram,

I finally committed a patch in revision 1762151 for your issue (it fixes the issue where you explicitly remove an instance-bound dependency from "destroy" method).

added test case in FELIX5274_CleanupInstanceBoundDependenciesInDestroy.java.

Let me explain what was going on (I prefer to write here all the internals; for the record):

So, not easy to explain, but the DM state machine is currently re-entrant, and this may causes some side effects when you add or remove dependencies from lifecycle callback methods.
In your case, you are removing an instance-bound dependency from the "destroy" method. So, at the time you remove the instance-bound dependency from the destroy() method, then the
DM state machine (which is currently transiting from INSTANTIATED_AND_WAITING_FOR_REQUIRED -> WAITING_FOR_REQUIRED) then re-enter in the "handleChange" method, which then transits from
WAITING_FOR_REQUIRED -> INACTIVE, and the problem is that at this point we have not yet finished the transition from INSTANTIATED_AND_WAITING_FOR_REQUIRED -> WAITING_FOR_REQUIRED.

Here is the scenario which helps to understand the issue:

1) you remove a component, using "dm.remove(component)" for example.

2) so, at this point, the "handleChange()" method (which is internal to ComponentImpl.java in DM) then performs some state transitions in order to destroy the component.

3) and at the time the state machine transits from INSTANTIATED_AND_WAITING_FOR_REQUIRED -> WAITING_FOR_REQUIRED, then it does the following:

{code}
        if (oldState == ComponentState.INSTANTIATED_AND_WAITING_FOR_REQUIRED && newState == ComponentState.WAITING_FOR_REQUIRED) {
            invoke(m_callbackDestroy);  // 3.1
            removeInstanceBoundDependencies();
            invokeRemoveRequiredDependencies(); // 3.2
            notifyListeners(newState);
            if (! someDependenciesNeedInstance()) {
                destroyComponent();
            }
            return true;
        }
{code}

4) in 3.1, the "invoke(m_callbackDestroy)" is invoked, and your "destroy" callback then removes an instance bound dependency that was initially added in "init" callback.
And at this point the DM state machine sudenly transits from WAITING_FOR_REQUIRED -> INACTIVE (notice it has not yet finished the initial transition):
{code}
        if (oldState == ComponentState.WAITING_FOR_REQUIRED && newState == ComponentState.INACTIVE) {
            stopDependencies();
            destroyComponent(); // 4.1
            notifyListeners(newState);
            return true;
        }
{code}

so, you see in 4.1 the component is destroyed, and when the initial transition resumes, when we execute 3.2 the component
has already been destroyed in 4.1, and required dependencies can't be unbounded.

So, I think that the only way to properly resolve this issue without risks is to not allow reentrency in the state machine, a transition should be fully finished before evaluating what has been done in a lifecyle callback.

the patch which avoid the state machine to be reentrant is located at the beginning of the handleChange method:

{code}
    private void handleChange() {
        if (isHandlingChange()) {
            return;
        }
        ...
    }
{code}

With the patch, all tests are passing OK, except the ComponentTest.testAddAvailableDependencyFromInitCallback().

This test was testing that a dependency added or removed from a lifecycle callback was immediately evaluated, but now, since the patch does not allow to re-enter into the state machine, I had to modify a bit the test, which is still making sense.






> remove callback fails after manually removing dynamic dependencies
> ------------------------------------------------------------------
>
>                 Key: FELIX-5274
>                 URL: https://issues.apache.org/jira/browse/FELIX-5274
>             Project: Felix
>          Issue Type: Bug
>          Components: Dependency Manager
>    Affects Versions: org.apache.felix.dependencymanager-r8
>            Reporter: Bram Pouwelse
>            Assignee: Pierre De Rop
>
> When dynamic instance bound dependencies are removed in the component destroy dm fails to call the remove callback. 
> I use an adapter service like 
> {code}
> manager.add(createAdapterService(ApplicationService.class, null, "onAdd", "onChange", "onRemove")
>   .setInterface(Servlet.class.getName(), null)
>   .setImplementation(WinkServlet.class)
>   .setCallbacks("dmInit", "dmStart", "dmStop", "dmDestroy")
>   );
> {code}
> In the WinkServlet init callback an additional dependency is added and this is removed in the destroy callback. When the component is removed I get log messages like
> {code}
> ERROR org.amdatu.web.rest.wink - [main] "onRemove" callback not found on component instances []
> {code}
> [http://www.mail-archive.com/users%40felix.apache.org/msg17300.html]



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)