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

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

In order to better understand what is going on and to better verify that the 
attached patch is working, I just committed a new test case, which seems to 
reproduce the issue more reliably.

The itest is made of the following parts:

* 
org.apache.felix.dependencymanager.ds.itest/resource/integration_test_FELIX_4984.xml:
 this file defines two components A, and B. A has a dynamic 1..1 dependency on 
B. And B has a 0..N dynamic dependency on A.

* 
org.apache.felix.dependencymanager.ds.itest/src/org/apache/felix/scr/integration/components/felix4984/A.java:
 This is the A component that is defined in the integration_test_FELIX_4984.xml 
DS descriptor.

* 
org.apache.felix.dependencymanager.ds.itest/src/org/apache/felix/scr/integration/components/felix4984/B.java:
 this is the B component that is defined in the integration_test_FELIX_4984.xml 
DS descriptor.

* 
org.apache.felix.dependencymanager.ds.itest/src/org/apache/felix/scr/integration/Felix4984Test.java:
 This is the new  integration test that performs a loop (1000 times). During 
each iteration, the tiny bundle that corresponds to the 
integration_test_FELIX_4984.xml descriptor is restarted. Then the test is 
getting a B instance from the B reference. Finally, the test is then getting a 
A instance from the A reference.

So, using the test above, and without your patch, I think that the issue is 
reproduced reliably: after a couple of iterations, A.setB(B b) is called twice 
on the same b instance.
I attached 
org.apache.felix.scr.integration.Felix4984Test.test_A11_B0n_delayed_B_first_ABoundToAtMostOneB.log
and if you take a look at line 3100 (during iteration #16), you will see the 
two stacktraces when A.setB(B b) is called.

A.setB(B b) is first called when we dereference the A service reference (see 
first stacktrace, line 3104).
A.setB(B b) is called a second time because previously, when the B service 
reference was dereferenced, then a task was scheduled in the ComponentActor 
thread, which calls invokeBindMethodLate (see second stacktrace, line 3147).

So, using your patch seems to resolve the problem (I'm not sure enough to say 
if it's really necessary to ensure that all references are satisfied before 
scheduling the "bind later" tasks).

But, using the new integration test, a new problem occurs. It's probably less 
important than the previous one but it's worth taking a look at it. I think 
that the task scheduled in the component actor thread that calls 
invokeBindMethodLate has a kind of synchronization issue, because in the test, 
when we stop the tiny bundle (in line 69, in Felix4984Test.java), the task 
running in the actor thread is concurrently invoking  invokeBindMethodLate, 
which is then getting a NPE.

I have attached a new log file 
org.apache.felix.scr.integration.Felix4984Test.test_A11_B0n_delayed_B_first_ABoundToAtMostOneB-NPE-with_patch.log

Please take a look at line 78622 and check the exception:

{code}
  78621 L=1 D=15:59:30,8 T="SCR Component Actor" (log Unexpected): Unexpected 
problem executing task Late binding task of reference 
[org.apache.felix.scr.integration.components.felix4984.A] for         
dependencyManagers [org.apache.felix.scr.impl.ComponentRegistry$Entry@4b21844c]
  78622 java.lang.NullPointerException
  78623         at 
org.apache.felix.scr.impl.helper.BindMethod.getServiceObject(BindMethod.java:643)
  78624         at 
org.apache.felix.scr.impl.manager.DependencyManager.getServiceObject(DependencyManager.java:2134)
  78625         at 
org.apache.felix.scr.impl.manager.DependencyManager.doInvokeBindMethod(DependencyManager.java:1648)
  78626         at 
org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethod(DependencyManager.java:1633)
  78627         at 
org.apache.felix.scr.impl.manager.SingleComponentManager.invokeBindMethod(SingleComponentManager.java:370)
  78628         at 
org.apache.felix.scr.impl.manager.DependencyManager.invokeBindMethodLate(DependencyManager.java:1577)
  78629         at 
org.apache.felix.scr.impl.ComponentRegistry$1.run(ComponentRegistry.java:570)
  78630         at 
org.apache.felix.scr.impl.ComponentActorThread.run(ComponentActorThread.java:99)
  78631         at java.lang.Thread.run(Thread.java:745)
{code}


> 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
>
>
> 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