Announcement

**Isomorphic** · 9 Jul 2015, 13:47

What this suggests is that you've created a server-side SimpleType definition (.type.xml file) with a very generic name - something like "field" or "type". This is causing this SimpleType to be invoked when processing DataSource XML, specifically, it's being invoked to process the XML of your dynamically generated DataSource definitions. Because this SimpleType in turn appears to have a validator that can actually cause a DataSource request (to look up stored values), you end up in a deadlock.

Most likely, you didn't intend to create a SimpleType that would get involved in interpreting DataSource XML *definitions*, just in validating DataSource records.

Can you look over your SimpleTypes and identify which one this is?

What we might do to prevent this in the future is introduce a concept of a SimpleType that is only used in the narrow role of validation DSRequest data and not in other data transforms and XML interpretation.

**godonnell_ip** · 9 Jul 2015, 14:11

As always, a quick and comprehensive response! Thank you very much for your feedback. We will have a look and report back our findings.

**godonnell_ip** · 9 Jul 2015, 23:18

So, it turns out that we don't have any server side SimpleTypes defined.

However, your answer prompted me to dig deeper into the stack traces I posted above and it became apparent to me that threads http-bio-8088-exec-68 and http-bio-8088-exec-69 are the ones that are in contention which caused me to further investigate.

I initiated the logic that triggered both stack traces independently, starting with the functionality in http-bio-8088-exec-68. It executed cleanly and didn't have any issues.

I then moved onto http-bio-8088-exec-69, which was always more suspect since it is triggered during login and is where we load all necessary datasources. By setting breakpoints, I determined that only one datasource caused a traversal of the specific lines detailed in the stack trace. Specifically, com.isomorphic.velocity.StoredRecordHandler.fetchStoredRecord() was invoked several times while processing this datasource.

Further tracing allowed me to identify that it appeared to be when processing the customSelectExpression elements on several field definitions. However, other fields have customSelectExpressions and the affected fields also have customInsertExpressions and customUpdateExpressions, and these did not cause the code to traverse this method.

Based on my observations during further tracing, it seems that the com.isomorphic.velocity.StoredRecordHandler.fetchStoredRecord() method is only called when the field definition looks like this (note the placement of the filterFields and pickListFields elements):

Code:

		<field name="concept" nativeName="CONCEPT" title="Concept" type="integer" primaryKey="true" canEdit="false"
			optionDataSource="IPCNCPT" valueField="CCNCPT" displayField="CNAME"
			release="4.0" editorType="ComboBoxItem" completeOnTab="true">
			<filterFields>CNAME</filterFields>
			<filterFields>CCNCPT</filterFields>
			<pickListFields>
				<ListGridField name="CNAME" width="150"/>
				<ListGridField name="CCNCPT" width="50"/>
			</pickListFields>
			<customSelectExpression>
				<![CDATA[CASE WHEN CONCEPT='' THEN NULL ELSE CAST(CONCEPT AS INTEGER) END]]>
			</customSelectExpression>
			<customInsertExpression>
				<![CDATA[#if($values.concept && $values.concept!=0 && $values.division==0)
							digits(cast($values.concept as dec(4,0))) 
						#else 
							' ' 
						#end]]>
			</customInsertExpression>
			<customUpdateExpression>
				<![CDATA[#if($values.concept && $values.concept!=0 && $values.division==0)
							digits(cast($values.concept as dec(4,0))) 
						#else 
							' ' 
						#end]]>
			</customUpdateExpression>			
		</field>

If I instead alter the definition of those fields as follows, the method is not traversed:

Code:

		<field name="concept" nativeName="CONCEPT" title="Concept" type="integer" primaryKey="true" canEdit="false"
			optionDataSource="IPCNCPT" valueField="CCNCPT" displayField="CNAME"
			release="4.0" editorType="ComboBoxItem" completeOnTab="true">
			<customSelectExpression>
				<![CDATA[CASE WHEN CONCEPT='' THEN NULL ELSE CAST(CONCEPT AS INTEGER) END]]>
			</customSelectExpression>
			<customInsertExpression>
				<![CDATA[#if($values.concept && $values.concept!=0 && $values.division==0)
							digits(cast($values.concept as dec(4,0))) 
						#else 
							' ' 
						#end]]>
			</customInsertExpression>
			<customUpdateExpression>
				<![CDATA[#if($values.concept && $values.concept!=0 && $values.division==0)
							digits(cast($values.concept as dec(4,0))) 
						#else 
							' ' 
						#end]]>
			</customUpdateExpression>			
			<filterFields>CNAME</filterFields>
			<filterFields>CCNCPT</filterFields>
			<pickListFields>
				<ListGridField name="CNAME" width="150"/>
				<ListGridField name="CCNCPT" width="50"/>
			</pickListFields>
		</field>

Does this make sense to you? Is there something we have overlooked in our field definitions?

Perhaps I should point out that the optionDataSources these fields refer to have a checkRelatedDS element which is implemented as discussed in this archived forum post: http://forums.smartclient.com/archiv...hp/t-8644.html. Since these datasources are all part of a hierarchical structure, each level has a checkRelatedDS relationship to the next level down in the hierarchy.

If it will help I can post the full datasource definitions. In the meantime, we will attempt to see whether just making this change is sufficient to prevent the problems we have been observing.

**Blama** · 10 Jul 2015, 01:55

Hi godonnell_ip,

I don't know if this is the reason, but with respect to the DataSourceField-.ds.xml-docs, the following are not allowed in your definition:
optionDataSource, valueField, release, completeOnTab, filterFields, pickListFields, ListGridField

They might be allowed in a type definition and you can have your .ds.xml point to that type then.

displayField is allowed, but does something else than you might be thinking. You most likely wanted to use it as attribute for your optionDataSource, but it has an other meaning in .ds.xml.

Best regards
Blama

**godonnell_ip** · 10 Jul 2015, 07:18

Thanks for the suggestion, Blama.

However, my understanding is that properties defined on DataSourceFields that are not part of the DataSourceField schema are ignored and simply passed through as attributes to objects that are created through binding to that DataSource.

In other words, when a ComboBoxItem is created for the CONCEPT field I showed as an example, the optionDataSource, valueField, pickListFields and filterFields attributes drive the configuration of that item.

Some of the others you mentioned (e.g. release and completeOnTab) are custom properties that we have added. Release is used when loading the DataSource (we have registered a DynamicDSGenerator) to determine whether or not to include the field depending on the version of the underlying database we are using.

So, while you may be right, we have many datasource definitions that rely on this behavior and that have done for years. I am hoping very strongly that we are not relying on behavior that has been deprecated.

Thanks again for taking the time to make the suggestion, though. I will ponder a little more and let you know where we end up.

**godonnell_ip** · 10 Jul 2015, 18:27

From what I observed last night researching the deadlock, as well today looking at a general slowing of the application over time, it seems that many calls are made to DataSourceManager.getDataSource().

This is interesting, since I just came across this (which I'm afraid was news to me...)

"Note that these [DataSourceManager] APIs are only intended for use in cases where your code is not running in the context of an RPCManager, such as a non-servlet application or some init-time logic. For normal client-server transaction code that has access to an RPCManager, use RPCManager.getDataSource() instead; this API tracks and frees all DataSource instances it returns at the end of the transaction cycle, so you don't need to worry about your application code leaking DataSource objects."
http://www.smartclient.com/smartgwtee-5.0/server/javadoc/com/isomorphic/datasource/DataSourceManager.html

And this...

"[RPCManager.getDataSource(dsName)] Returns an instance of the DataSource named in the "dsName" parameter. This method borrows an object from the framework's DataSource pool, and ensures that it is freed at the end of the request cycle. It is the recommended way to obtain an arbitrary DataSource object in your own server-side code. Note that this method is intended for use if you need to obtain some arbitrary DataSource. If what you want is the DataSource associated with the current DSRequest (a common use case), use DSRequest.getDataSource() instead. Also, if you are trying to access some arbitrary DataSource purely because you need to run a DSRequest on it (another very common use case), consider just creating the DSRequest instead, using one of the constructors that accepts a DataSource name."
http://www.smartclient.com/smartgwtee-5.0/server/javadoc/com/isomorphic/rpc/RPCManager.html#getDataSource(java.lang.String)

We call DataSourceManager.getDataSource() (and it's synonym DataSourceManager.get()) almost as a matter of course in our serverConstructor classes, whereas we do actually have a convenience method (IpDataSource.newDataSourceObject()) in the superclass from which they are derived. This method does just about what is described above, although in what looks to be a little roundabout fashion:

Code:

      public static DataSource newDataSourceObject(String dataSourceName,
                  DSRequest req) throws Exception {
            DSRequest newRequest = newRequestInContext(req, dataSourceName,
                        DataSource.OP_FETCH);
            return newRequest.getDataSource();
      }

I'm thinking this could be simplified to:

Code:

      public static DataSource newDataSourceObject(String dataSourceName,
                  DSRequest req) throws Exception {
            return req.getRPCManager().getDataSource(dataSourceName);
      }

This method is used in some of our older datasource logic, so I guess this was known at the time that stuff was written but we seem to have fallen into bad habits.

I wonder if you could comment on whether this may be our problem? The mention of "your application code leaking DataSource objects" makes me think it's something we should take seriously, especially given our current situation.

I'll be doing a bit of search and replace, as well as refactoring of methods that will require the request to be passed into them where necessary and will then run some more profiling locally. Let's see where that takes us, but I'd be interested in your comments in the meantime.

Thanks in advance.

**Isomorphic** · 13 Jul 2015, 14:18

OK, multiple points to discuss:

1. DataSourceManager.getDataSource() - yes, you are leaking DataSources, probably at a prodigious rate, and your code should be corrected along the lines you suggest.

2. Blama is correct that definitions that are not documented for DataSourceField - such as your <pickListFields> - are not allowed there. We didn't expect you'd be doing this, which is why we assumed you have custom SimpleTypes, which would have created similar stack traces.

As far as what actually happens when you do this - the actual behavior (which is undocumented and unsupported) is that such settings will be applied to *all* DBCs that use the DataSource. So when you, for example, bind a TileGrid to this DataSource, it gets pickListFields, filterFields, etc. TileGrids have no behaviors for such settings for now, but could in the future, and the behavior could be something you didn't intend.

So even if you are willing to rely on undocumented behavior, this is a high-risk practice in terms of future-proofing.

It also created this deadlock - another reason it's a high-risk thing to do, since we don't test undocumented usage. We're still looking at the deadlock issue since it looks like there's a related bug, but it's not clear that fixing the related bug will solve your deadlock, and we have not identified any supported usage that could cause a similar deadlock.

The correct and supported thing to do here is to put the settings that are intended for the ComboBoxItem into a (trivial) ComboBoxItem subclass, and use the editorType setting to cause that ComboBoxItem to be used.

**godonnell_ip** · 14 Jul 2015, 08:14

Thank you again for your reply.

I went through the code after my post on Friday and either corrected all the calls to DataSourceManager.getDataSource() or explicitly freed them when called in other contexts.

We have also reviewed, removed and refactored synchronization that we use while building the datasources in our dynamic datasource generator in case our own code was contributing to the deadlocks. We are still seeing the same behavior (application is fast at first, but over a short period of time starts to respond slowly, and finally RPC requests just time out).

We have observed the application through VisualVM, which has helped identify the deadlocks (we experienced another yesterday in which SQLDataSourceManager was deadlocked with DataSourceManager).

At this point we are having a real problem coming up with a real answer. I know it's impossible for you to conjecture on a large application without seeing it first hand and we very much appreciate the guidance you are able to provide.

Please keep the suggestions coming if more occur to you. Be assured that we will keep you posted of further developments.

I guess our next stop is to look at all of the datasource definitions we have that are abusing undocumented definitions.

**Isomorphic** · 14 Jul 2015, 12:26

There's the existing suggestion to correct your usage - replacing properties that are not allowed under DataSourceField with editorType instead. That will definitely fix one category of deadlocks, and might possible correct your cumulative slowdown.

Secondly we would recommend adding some logging or counting logic to see if your DataSources continue to be allocated indefinitely, or if you instead see the normal behavior of the framework - a pool is created and stops growing.

**godonnell_ip** · 15 Jul 2015, 08:58

Thanks - yes, we do need to review our usage of undocumented attributes on our DataSourceFields (thank you again for that).

However - the good (although embarrassing) news is that one of the thread dumps we took yesterday while observing a deadlock showed us that DataSourceManager.getDataSourceFromStack() was calling itself multiple times recursively.

Knowing how the dynamic datasource generator logic will work its way through a stack of registered generators, this led us to review the code where we register our generator and (ugh!) found that we were adding it regardless of whether we had already added it previously. Consequently, the stack would grow as more and more requests were made, and it would take longer and longer to work through the stack.

Note that we did not observe this in versions of the code running on earlier versions of the libraries, but we note that at least DataSourceManager.getDataSource() is now synchronized, so perhaps the newer libraries were causing more and more blocking to occur (i.e. slowing down responses) until a deadlock occurred (i.e. responses stopped).

Having corrected the code yesterday and run through about 18 hours of testing, we feel that this was indeed the cause of the issue.

We will still need to review our datasource definitions, but I think we have at least got past the critical problem that was holding up any reasonable functional testing.

Thanks so much to you and Blama for reading this thread and providing your suggestions. I almost wish it had ended up been a bug in the library so we could be blameless, but as long as we humans keep making mistakes in our code the machines won't be able to take over... :-)

**Isomorphic** · 15 Jul 2015, 14:29

No problem, all code has bugs :) Glad you resolved it!

Announcement

Potential Thread Deadlock Within DataSourceManager.getDataSource()

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment

Comment