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

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

Ok, the NPE is caused by the fact that the 
test_A11_B0n_delayed_B_first_ABoundToAtMostOneB() is getting a A instance and 
is then immediately stopping the bundle:

{code}
            ServiceReference[] serviceReferencesA = 
bundleContext.getServiceReferences( A.class.getName(), "(service.pid=" + 
componentNameA + ")" );
            TestCase.assertEquals( 1, serviceReferencesA.length );
            ServiceReference serviceReferenceA = serviceReferencesA[0];
            Object serviceA = bundleContext.getService( serviceReferenceA );
            assertNotNull( serviceA );

            A a = getServiceFromConfiguration(componentA, A.class);
            assertABoundToOneB(a);

            bundle.stop();
{code}

So, when A is dereferenced, the invokeBindMethodLate() call is scheduled in the 
component actor thread in order to call B.setA(A a).
But since we immediately stop the bundle, then we are getting the NPE in the 
invokeBindMethodLate() method.

Adding a short delay just before the bundle.stop() method would avoid the NPE.

All this is now a corner case, but do you think it's necessary to add a 
protection somewhere in order to avoid the NPE ? It looks like if we add the 
following null check in the (already patched) DependencyManager.java, at line 
1567, then we can handle such concurrent situation and avoid the NPE (see the 
null check on the refPair at the end of the following sample code):

{code}
    public void invokeBindMethodLate( final ServiceReference<T> ref, int 
trackingCount )
    {
        if ( !isSatisfied() )
        {
            return;
        }
        if ( !isMultiple() )
        {
            Collection<RefPair<S, T>> refs = m_customizer.getRefs( new 
AtomicInteger( ) );
            if (refs.isEmpty())
            {
                return;
            }
            RefPair<S, T> test = refs.iterator().next();
            if ( ref != test.getRef())
            {
                //another ref is now better
                return;
            }
        }
        //TODO dynamic reluctant
        RefPair<S, T> refPair = m_tracker.getService( ref );
        if (refPair == null) {
            return; // the tracker has been probably closed ?
        }
        ...
{code}

(not really sure the above patch can fix the NPE in all situations ...)


> Issues in CircularReferenceTest
> -------------------------------
>
>                 Key: FELIX-4984
>                 URL: https://issues.apache.org/jira/browse/FELIX-4984
>             Project: Felix
>          Issue Type: Bug
>          Components: Declarative Services (SCR)
>            Reporter: Pierre De Rop
>            Priority: Minor
>             Fix For: scr-2.0.2
>
>         Attachments: felix-4984.diff, 
> org.apache.felix.scr.integration.CircularReferenceTest.test_A11_B0n_delayed_B_first.log,
>  
> org.apache.felix.scr.integration.Felix4984Test.test_A11_B0n_delayed_B_first_ABoundToAtMostOneB-NPE-with_patch.log.gz,
>  
> org.apache.felix.scr.integration.Felix4984Test.test_A11_B0n_delayed_B_first_ABoundToAtMostOneB.log
>
>
> This issue is described in the dev mailing list, in 
> http://www.mail-archive.com/[email protected]/msg37281.html
> while working on FELIX-4955, I sometimes have the CircularReferenceTest 
> failing.
> Everything is located in my sandbox, in 
> http://svn.apache.org/repos/asf/felix/sandbox/pderop/dependencymanager.ds/
> To reproduce the test:
> install eclipse Mars
> install latest bndtools using "install new software" from Eclipse, and then 
> add latest stable release from http://dl.bintray.com/bndtools/bndtools/latest/
> install a java8 runtime (I'm using oracle java8 1.8.0_45, 64 bit version). 
> The whole new dependencymanager.ds project is intented to be build in java8.
> checkout my sandbox:
> $ svn checkout 
> http://svn.apache.org/repos/asf/felix/sandbox/pderop/dependencymanager.ds
> go to "dependencymanager.ds" directory:
> $ cd dependencymanager.ds/
> due to a pending issue, you have to first build the DM bnd annotation plugin 
> before importing the project into eclipse. to do so, just type:
> $ ./gradlew org.apache.felix.dependencymanager.annotation:jar
> now launch eclipse and use the the dependencymanager/ds directory as the 
> workspace dir for Eclipse.
> switch to BndTools perpective.
> import the bndtools project into eclipse: Import -> Existing Projects into 
> Workspace -> Browse -> select dependencymanager.ds directory (it is proposed 
> by default).
> normally, and hopefully, everything should compile fine. Junit tests are left 
> in org.apache.felix.dependencymanager.ds/ directory and integration tests are 
> located in org.apache.felix.dependencymanager.ds.itest/ directory.
> Open under Eclipse the 
> org.apache.felix.dependencymanager.ds.itest/src/org/apache/felix/scr/integration/CircularReferenceTest.java
> I slightly modified it in order to dump stack traces when A component is 
> bound multiple times to the same B instance.
> (I believe that only delayed components are concerned by the issue).
> For example, in the test_A11_B0n_delayed_A_first() method, I added a call to 
> "assertABoundToOneB(a)" like this:
> {code}
>     @Test
>     public void test_A11_B0n_delayed_A_first() throws InvalidSyntaxException
>     {
>         String componentNameA = "4.1.A.1.1.dynamic";
>         final ComponentConfigurationDTO componentA = 
> findComponentConfigurationByName( componentNameA, 
> ComponentConfigurationDTO.SATISFIED );
>         String componentNameB = "4.1.B.0.n.dynamic";
>         final ComponentConfigurationDTO componentB = 
> findComponentConfigurationByName( componentNameB, 
> ComponentConfigurationDTO.SATISFIED );
>         delay();
>         A a = getServiceFromConfiguration(componentA, A.class);
>         assertABoundToOneB(a);
>         delay(); //async binding of a to b after circular ref detected
>         B b = getServiceFromConfiguration(componentB, B.class);
>         assertEquals( 1, b.getAs().size() );
>     }
> {code}
> the "assertABoundToOneB(a)" call does this:
> {code}
>     private void assertABoundToOneB(A a) {
>         if (a.getBs().size() != 1) {
>             System.err.println("detected problem ...");
>             a.dumpStackTracesWhenBWasBound();
>         }
>         assertEquals( 1, a.getBs().size());
>     }
> {code}
> And stacktraces will be dumped in order to determine why A was bound two 
> times to the same B instance.
> it's possible that you have to run several times the "CircularReferenceTest" 
> test before having a failure (and some stacktraces).
> Thanks.



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

Reply via email to