search

osate / osate2

Open Source AADL2 Tool Environment
http://osate.org
Eclipse Public License 2.0
39 stars 8 forks source link

Some data-access connections are not being instantiated. #2161

Closed schwerdf closed 4 years ago

schwerdf commented 4 years ago

Summary

When instantiating the AADL model below using the latest development version of OSATE, a data-access connection within the model is not being instantiated.

Expected and Current Behavior

When instantiating this model with OSATE 2.6.1, the instance file contained a connection instance corresponding to the data-access connection s1:

    <connectionInstance name="shared -> t1.shared" complete="true" kind="accessConnection" destination="//@componentInstance.0/@componentInstance.1/@featureInstance.0" source="//@componentInstance.0/@componentInstance.0">
      <connectionReference context="//@componentInstance.0" source="//@componentInstance.0/@componentInstance.0" destination="//@componentInstance.0/@componentInstance.1/@featureInstance.0">
        <connection xsi:type="aadl2:AccessConnection" href="../DataAccessConnectionTest.aadl#/0/@ownedPublicSection/@ownedClassifier.7/@ownedAccessConnection.0"/>
      </connectionReference>
    </connectionInstance>

Steps to Reproduce

  1. With the AADL model below, instantiate the system implementation DACT.impl.
  2. Open the instance file.
  3. Note that there is no connection instance of any kind in the file.
package DataAccessConnectionTest
public
    data Shared     
    end Shared;

    data implementation Shared.impl
    end Shared.impl;

    subprogram RequiresData
    features
        shared: requires data access Shared;
    end RequiresData;

    subprogram implementation RequiresData.impl
    end RequiresData.impl;

    thread ThreadRequiresData
    features
        shared: requires data access Shared;            
    end ThreadRequiresData;

    thread implementation ThreadRequiresData.impl
    subcomponents
        sp: subprogram RequiresData.impl;
    calls main: {
        s1 : subprogram sp;
    };      
    connections
        dc: data access shared -> sp.shared;
    end ThreadRequiresData.impl;        

    process Proc    
    features
        externalData: requires data access;                 
    end Proc;   

    process implementation Proc.impl
    subcomponents
        shared: data Shared.impl;
        t1: thread ThreadRequiresData.impl;
    connections
        s1: data access shared -> t1.shared;
    end Proc.impl;  

    processor P
    end P;

    processor implementation P.impl
    end P.impl;

    system DACT
    end DACT;

    system implementation DACT.impl
    subcomponents
        proc: processor P.impl;
        app: process Proc.impl;
    properties
        Actual_Processor_Binding => (reference (proc)) applies to app;      
    end DACT.impl;      

end DataAccessConnectionTest;

Environment

AaronGreenhouse commented 4 years ago

This problem is caused by the connection dc in ThreadRequiresData.impl. If you remove it, then the connection instance is generated.

This is almost certainly caused by my fix to Issue #2032. I think the problem is that the connection is seen as terminating at a subprogram, and we were trying to treat those kinds of connections specially. I may have overlooked the fact that subprograms can have access connections.

AaronGreenhouse commented 4 years ago

The problem is in CreateConnectionsSwitch.appendSegment(). Because of the connection to the subprogram, the conns list at line 662 is non-empty. SO we go to the else-branch at line 696. We go through the loop at 722 and call appendSegment() recursively at line 730. Then we get to line 408 in the recursive call and return without doing anything because the connection is a DataAccess connection in a subprogram.

Need to weed this out before hand in the checks around line 661 I think.

AaronGreenhouse commented 4 years ago

Changed the filter sent to getIngoingConnections() at line 662 to 

                        List<Connection> conns = AadlUtil.getIngoingConnections(toImpl, toFeature,
                                c -> {
                                    if (c instanceof AccessConnection) {
                                        /*
                                         * Bug 2161: Ignore internal access connections if they go to a subprogram. NB. access
                                         * connection can be written in either direction, so the subprogram could be the source
                                         * or destination context.
                                         */
                                        if (c.getAllSourceContext() instanceof SubprogramSubcomponent
                                                || c.getAllDestinationContext() instanceof SubprogramSubcomponent) {
                                            hasIgnoredConnection.set(true);
                                            return false;
                                        } else {
                                            return true;
                                        }
                                    } else if (c instanceof ParameterConnection) {
                                        // always ignore parameter connections
                                        hasIgnoredConnection.set(true);
                                        return false;
                                    } else {
                                        // Ignore other connections only if the component is connection ending
                                        if (finalComponent) {
                                            hasIgnoredConnection.set(true);
                                            return false;
                                        } else {
                                            return true;
                                        }
                                    }
                                });

Fixes the problem above, but causes some of the tests for 2032 to fail. Need to check them out.

AaronGreenhouse commented 4 years ago

Two of the 3 tests from 2032 that broke are tests that seem to rely on the connection instance connecting to a subprogram subcomponent:

The question I have is this: was the fix in 2032 incomplete? I'm pretty sure we should never have a connection that ends at a subprogram subcomponent. (The problem reported here in 2161 is that we are correctly not connecting to a subprogram, but we didn't record correctly the fact that we should connect to the container of the subcomponent.) I don't think I deliberately allowed the above cases in 2032. I merely captured the reality of what was allowed. Somehow I missed that this cases probably shouldn't be allowed. They feel through the cracks, I think, because the subprogram is not in a thread, process, or processor, but in a data subcomponent.

So the big question is: Do we ever want a connection instance to end at a subprogram?

AaronGreenhouse commented 4 years ago

I went back the master branch to play around with this some more. I have a new observation: If I change the connections to be bidirectional, then the connection instance is created and follows both connections (dc and s1).

It seems that the author of this issue expects that the connection only contain s1.

stevevestal commented 4 years ago

Some background.

The AADL standard only allows Concurrency_Control_Protocol to be declared on a data sub-component. This is a necessary property to deal with semaphore protocols (not shown in this simplified model to reproduce the issue). We need a data access connection to go from the data sub-component itself down to the "requires access" to that data object. We wrap that inside a subprogram call because that is the only standard way we could think of to specify a worst-case locking time. Analysis tools that handle real-time semaphores need this information, and need it on both ends to do semaphore protocol consistency checking.

There are provides and requires access to subprogram and subprogram group declarations in AADL, but that is different than provides and requires access to data.

AaronGreenhouse commented 4 years ago

Observed yesterday that something seems to be broken with unidirectional access connections. There are cases where the unidirectional connection is not created, but the bidirectional connections are.

Spent the day creating a bunch of test cases that explore this across a number of axes:

This gives me 12 AADL packages. Within each one, I have three system variants where the shared subcomponent and the shared access feature are connected in one of three ways:

  1. With a bidirectional connection
  2. With a unidirectional connection from the shared component to the feature
  3. With a unidirectional connection from the feature to the shared component

All cases worked as expected, except for the cases where we have a data subcomponent being connected to a data access feature in subprogram. This bug report covers some of those cases, but not all of them.

The tests are being run on the "master" branch.

(more details to follow)

AaronGreenhouse commented 4 years ago

The simplest case that fails is when we connect a data subcomponent to a data access feature of a sibling subprogram component:

package SharedData_to_Subprogram_Peers
public
    data ShareMe
    end ShareMe;

    subprogram S
        features
            daf: requires data access ShareMe;
    end S;

    system Root
    end Root;

    system implementation Root.generic
        subcomponents
            shareMe: data ShareMe;
            s: subprogram S;
    end Root.generic;

    system implementation Root.bidirectional extends Root.generic
        connections
            dac: data access shareMe <-> s.daf;
    end Root.bidirectional;

    system implementation Root.fromSharedComponent extends Root.generic
        connections
            dac: data access shareMe -> s.daf;
    end Root.fromSharedComponent;

    system implementation Root.toSharedComponent extends Root.generic
        connections
            dac: data access s.daf -> shareMe;
    end Root.toSharedComponent;
end SharedData_to_Subprogram_Peers;

If we change the subprogram to a data component or we change the shared data to a shared subprogram we get the connection instances that we expect.

(NB. This is on the master branch.)

AaronGreenhouse commented 4 years ago

The next case that doesn't work is when we connect a data subcomponent to a data access feature of a subprogram component and that subprogram is contained in a second data component:

package SharedData_to_Subprogram_Nested_in_Data
public
    data ShareMe
    end ShareMe;

    subprogram InnerS
        features
            daf1: requires data access ShareMe;
    end InnerS;

    data OuterD
        features
            daf2: requires data access ShareMe;
    end OuterD;

    data implementation OuterD.generic
        subcomponents
            inner: subprogram InnerS;
    end OuterD.generic;

    data implementation OuterD.bidirectional extends OuterD.generic
        connections
            dac1: data access daf2 <-> inner.daf1;
    end OuterD.bidirectional;

    data implementation OuterD.fromSharedComponent extends OuterD.generic
        connections
            dac1: data access daf2 -> inner.daf1;
    end OuterD.fromSharedComponent;

    data implementation OuterD.toSharedComponent extends OuterD.generic
        connections
            dac1: data access inner.daf1 -> daf2;
    end OuterD.toSharedComponent;

    system Root
    end Root;

    system implementation Root.generic
        subcomponents
            shareMe: data ShareMe;
            outer: data OuterD.generic;
    end Root.generic;

    system implementation Root.bidirectional extends Root.generic
        subcomponents
            outer: refined to data OuterD.bidirectional;
        connections
            dac2: data access shareMe <-> outer.daf2;
    end Root.bidirectional;

    system implementation Root.fromSharedComponent extends Root.generic
        subcomponents
            outer: refined to data OuterD.fromSharedComponent;
        connections
            dac2: data access shareMe -> outer.daf2;
    end Root.fromSharedComponent;

    system implementation Root.toSharedComponent extends Root.generic
        subcomponents
            outer: refined to data OuterD.toSharedComponent;
        connections
            dac2: data access outer.daf2 -> shareMe;
    end Root.toSharedComponent;
end SharedData_to_Subprogram_Nested_in_Data;

Here we never have any connection instances generated.

Again, if we change the subprogram to a data component, or the shared data to a shared subprogram things works correctly.

(NB. This is on the master branch.)

AaronGreenhouse commented 4 years ago

Finally, is the case where we connect a data subcomponent to a data access feature of a subprogram component and that subprogram is contained in a thread component:

package SharedData_to_Subprogram_Nested_in_Thread
public
    data ShareMe
    end ShareMe;

    subprogram InnerS
        features
            daf1: requires data access ShareMe;
    end InnerS;

    thread OuterT
        features
            daf2: requires data access ShareMe;
    end OuterT;

    thread implementation OuterT.generic
        subcomponents
            inner: subprogram InnerS;
    end OuterT.generic;

    thread implementation OuterT.bidirectional extends OuterT.generic
        connections
            dac1: data access daf2 <-> inner.daf1;
    end OuterT.bidirectional;

    thread implementation OuterT.fromSharedComponent extends OuterT.generic
        connections
            dac1: data access daf2 -> inner.daf1;
    end OuterT.fromSharedComponent;

    thread implementation OuterT.toSharedComponent extends OuterT.generic
        connections
            dac1: data access inner.daf1 -> daf2;
    end OuterT.toSharedComponent;

    process P
    end P;

    process implementation P.generic
        subcomponents
            shareMe: data ShareMe;
            outer: thread OuterT.generic;
    end P.generic;

    process implementation P.bidirectional extends P.generic
        subcomponents
            outer: refined to thread OuterT.bidirectional;
        connections
            dac2: data access shareMe <-> outer.daf2;
    end P.bidirectional;

    process implementation P.fromSharedComponent extends P.generic
        subcomponents
            outer: refined to thread OuterT.fromSharedComponent;
        connections
            dac2: data access shareMe -> outer.daf2;
    end P.fromSharedComponent;

    process implementation P.toSharedComponent extends P.generic
        subcomponents
            outer: refined to thread OuterT.toSharedComponent;
        connections
            dac2: data access outer.daf2 -> shareMe;
    end P.toSharedComponent;

    system Root
    end Root;

    system implementation Root.generic
        subcomponents
            myProcess: process P.generic;
    end Root.generic;

    system implementation Root.bidirectional extends Root.generic
        subcomponents
            myProcess: refined to process P.bidirectional;
    end Root.bidirectional;

    system implementation Root.fromSharedComponent extends Root.generic
        subcomponents
            myProcess: refined to process P.fromSharedComponent;
    end Root.fromSharedComponent;

    system implementation Root.toSharedComponent extends Root.generic
        subcomponents
            myProcess: refined to process P.toSharedComponent;
    end Root.toSharedComponent;
end SharedData_to_Subprogram_Nested_in_Thread;

Like the first bad example,

Again, if we change the subprogram to a data component, or the shared data to a shared subprogram things works correctly.

(NB. This is on the master branch.)

AaronGreenhouse commented 4 years ago

I'm not surprised the case where the the connection has to pass through a thread is different. I explicitly tested this case because I expected it would be. It's different because the connection generation code treats threads as "connection ending" and has different logic for them.

I'm not entirely surprised that ending at a subprogram might cause problems because there is a check that tries to eliminate access connections that end at subprograms (see one of the early comments above), but clearly it doesn't eliminate them in all case.

I'm not at all sure why it matters that it's data that is being shared.

AaronGreenhouse commented 4 years ago

This is a surprisingly simple problem. The culprit Is at line 392 in appendSegment() in CreateConnectionsSwitch:

        /*
         * Fix JD bug #222
         */
        if ((toEnd instanceof DataAccess) && (toEnd.getContainingClassifier() != null)
                && (toEnd.getContainingClassifier() instanceof SubprogramType)) {
            return;
        }

This is specifically dropping connections that go to data access features of subprogram subcomponents. I don't really know why this is here. It supposedly fixes Issue #222, but that report doesn't provide a lot of details of what the problem is that is supposedly being fixed, or why this is the correct way to fix it.

This is being a problem now for the specific example in the initial report here because before issue 2032, the connection instance was always terminated at the outer edge of the thread component. But after 2032, access connections are allowed to continue into the thread and connect to the subprogram. So now the above lines of code would kill the connection instance.

I have removed the above lines of code and they seem to fix the problem for the original example, and for the examples that I created above.

Need to run the regression tests and see if it messes anything else up.

NB. Undid the change from January 16th. That was incorrect.

AaronGreenhouse commented 4 years ago

Regression tests were okay, except for Issue2032Test.testJustData_Subprogram that tests exactly the case being described in this issue. The test expected there to be no connection instances because that was the old behavior. Updated it to expect two connection instances of length 3 (the connections are bidirectional and pass through a single layer of components).

need to create new Unit test drivers for this issue now.

lwrage commented 4 years ago

Does instantiation work, i.e., produce a reasonable instance model and report no errors, for the example from #222?

AaronGreenhouse commented 4 years ago

AH. Okay. The problem in Issue #222 is that the data component is being connected to a feature of a subprogram referenced as called subprogram in a call sequence. This should be filtereed out, but we need to allow the other cases.

Need to differentiate between the different uses of subprograms.

package test2
public
    with Base_Types;
    with Data_Model;

    subprogram job
        features
            param : requires data access Base_Types::Integer;
    end job;

    thread th end th;

    thread implementation th.impl
        subcomponents
            constantValue: data Base_Types::Integer
            {
                Data_Model::Initial_Value => ("3");
            };
        calls
            seq: { call1: subprogram job; };
        connections
            dac: data access constantValue -> call1.param;
    end th.impl;

    process proc end proc;

    process implementation proc.impl
        subcomponents
            th: thread th.impl;
    end proc.impl;

    system root end root;

    system implementation root.impl
        subcomponents
            proc: process proc.impl;
    end root.impl;
end test2;
AaronGreenhouse commented 4 years ago

Okay. Third try. Replaced the original check at line 392 with this one:

        /*
         * Fix JD bug #222
         */
        if ((toEnd instanceof DataAccess) && (toCtx instanceof SubprogramCall)) {
            return;
        }