Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
schwerdf opened this issue Jan 10, 2020 · 16 comments · Fixed by #2179
Closed

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

schwerdf opened this issue Jan 10, 2020 · 16 comments · Fixed by #2179

Comments

@schwerdf
Copy link
Contributor

@schwerdf schwerdf commented Jan 10, 2020

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

  • OSATE Version: 2.7.0-v20201007-1612
  • Operating System: Windows 7, macOS 10.14
@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 14, 2020

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 14, 2020

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 16, 2020

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 17, 2020

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

  • testJustSubprogramGroup_data
  • testJustSubprogramGroup_Data_FeatureGroup
    I haven't quite figured out what is going on with the 3rd test testShortAccessConnections_Cleanup yet, because that test is now generating more connections that it used to.

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 21, 2020

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

@stevevestal stevevestal commented Jan 22, 2020

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 22, 2020

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:

  • Whether the shared subcomponent is data or subprogram
  • Whether the subcomponent with the access feature is data or subprogram
  • The relationship of the shared subcomponent and the subcomponent with the access feature:
    1. They are siblings (peers) contained in the same component
    2. The shared subcomponent is a sibling to a data component that contains the subcomponent with the access feature.
    3. The shared subcomponent is a sibling to a thread component that contains the subcomponent with the access feature.

(The intent here is to see differences in treatment between "final" connection-ending components and components that are not connection-ending.)

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 23, 2020

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;
  • The bidirectional case only has a connection instance going to the data component.
  • The case where we declare a connection going from the data component has no connection instance.
  • The case where we declare a connection going to the data component does have the connection instance we expect.

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 23, 2020

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 23, 2020

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,

  • The bidirectional case only has a connection instance going to the data component.
  • The case where we declare a connection going from the data component has no connection instance.
  • The case where we declare a connection going to the data component does have the connection instance we expect.

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 23, 2020

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 24, 2020

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 24, 2020

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
Copy link
Contributor

@lwrage lwrage commented Jan 24, 2020

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

@AaronGreenhouse
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 24, 2020

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
Copy link
Contributor

@AaronGreenhouse AaronGreenhouse commented Jan 24, 2020

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;
		}
  • The connection instance is not created in the example from Issue #222.
  • The existing unit tests pass
  • My new examples (that are not created as unit tests yet) work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants