Grails

Domain get method intermittently returns incorrect object when called with a string

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Blocker Blocker
  • Resolution: Fixed
  • Affects Version/s: 1.0.1
  • Fix Version/s: 1.0.2
  • Component/s: None
  • Labels:
    None
  • Environment:
    Windows Intel Dual Core running VMWare 6.0
    AND
    MacBookPro (dual core)
  • Patch Submitted:
    Yes
  • Testcase included:
    yes

Description

The following call intermittently returns the wrong domain object when called with multiple threads:

<Domain Object>.get(params.id)

This seems to be fixed by the following change:

<Domain Object>.get(params.id.toInteger())

Have attached a simple project with a selenium IDE test (selenium-test.html) which demonstrates the issue. Failures are written to failed.txt in project root.
ExampleController contains variations of the call (some which fail, some which pass).

Run selenium-test.html to demonstrate the bug.

You can update start.gsp to call the relevant action and re-run selenium-test.html.

Have not managed to replicate this issue in an Integration test.

Attachments

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Graeme Rocher added a comment -

fixed

Show
Graeme Rocher added a comment - fixed
Hide
Permalink
Ola Bildtsen added a comment -

I noticed that the fix for this issue was to convert the instance variable SimpleTypeConverter to a thread-safe method variable in HibernateGrailsPlugin.groovy. But doesn't GetPersistentMethod have the same problem? When I was researching this issue on our own application, that's where I was digging around, the source looks like below (notice how the SimpleTypeConverter is an instance variable, which is not thread safe). I'm sure I don't understand fully why this code doesn't affect the problem, please enlighten me:

public class GetPersistentMethod extends AbstractStaticPersistentMethod {

private static final Pattern METHOD_PATTERN = Pattern.compile("^get$");
public static final String METHOD_SIGNATURE = "get";
private GrailsApplication application;
private SimpleTypeConverter typeConverter = new SimpleTypeConverter();

public GetPersistentMethod(GrailsApplication application, SessionFactory sessionFactory, ClassLoader classLoader) { super(sessionFactory, classLoader, METHOD_PATTERN); this.application = application; }

protected Object doInvokeInternal(final Class clazz, String methodName,
Object[] arguments) {
// if no arguments passed throw exception
if(arguments.length == 0)
throw new MissingMethodException(METHOD_SIGNATURE, clazz,arguments);
// if its not a map throw exception
Object arg = arguments[0];

if(arg == null)
return null;

GrailsDomainClass domainClass = (GrailsDomainClass) this.application.getArtefact(
DomainClassArtefactHandler.TYPE, clazz.getName());
if(domainClass != null) {
Class identityType = domainClass.getIdentifier().getType();

if(!identityType.isAssignableFrom(arg.getClass())) {
if(arg instanceof Number && Long.class.equals(identityType)) { arg = DefaultGroovyMethods.toLong((Number)arg); }
else { arg = typeConverter.convertIfNecessary(arg, identityType); }
}
}

return super.getHibernateTemplate().get( clazz, (Serializable)arg );
}

}

Show
Ola Bildtsen added a comment - I noticed that the fix for this issue was to convert the instance variable SimpleTypeConverter to a thread-safe method variable in HibernateGrailsPlugin.groovy. But doesn't GetPersistentMethod have the same problem? When I was researching this issue on our own application, that's where I was digging around, the source looks like below (notice how the SimpleTypeConverter is an instance variable, which is not thread safe). I'm sure I don't understand fully why this code doesn't affect the problem, please enlighten me: public class GetPersistentMethod extends AbstractStaticPersistentMethod { private static final Pattern METHOD_PATTERN = Pattern.compile("^get$"); public static final String METHOD_SIGNATURE = "get"; private GrailsApplication application; private SimpleTypeConverter typeConverter = new SimpleTypeConverter(); public GetPersistentMethod(GrailsApplication application, SessionFactory sessionFactory, ClassLoader classLoader) { super(sessionFactory, classLoader, METHOD_PATTERN); this.application = application; } protected Object doInvokeInternal(final Class clazz, String methodName, Object[] arguments) { // if no arguments passed throw exception if(arguments.length == 0) throw new MissingMethodException(METHOD_SIGNATURE, clazz,arguments); // if its not a map throw exception Object arg = arguments[0]; if(arg == null) return null; GrailsDomainClass domainClass = (GrailsDomainClass) this.application.getArtefact( DomainClassArtefactHandler.TYPE, clazz.getName()); if(domainClass != null) { Class identityType = domainClass.getIdentifier().getType(); if(!identityType.isAssignableFrom(arg.getClass())) { if(arg instanceof Number && Long.class.equals(identityType)) { arg = DefaultGroovyMethods.toLong((Number)arg); } else { arg = typeConverter.convertIfNecessary(arg, identityType); } } } return super.getHibernateTemplate().get( clazz, (Serializable)arg ); } }
Hide
Permalink
Martin Gee added a comment -

I'm having a simular issue with Grails 1.3.5 (I've tried the toInteger - doesn't work)...

Grails 1.3.5, MacOS, Java(TM) SE Runtime Environment (build 1.6.0_20-b02-279-10M3065)

In load testing my app I'm having a random issue with <domain>.get(id) returning null.

I create a domain object in a service and inturn (that same service) schedules a quartz job which immediately tries to read just newly created domain object (with a get(id) ). NOTE : these are 2 different threads...

Flow:
[http-8080-1-thread] <controller> -> my service -> create domain:save(flush:true), schedule quartz job
[quartzScheduler_Worker-10-thread] <quartz> -> job -> get domain object (id in jobmap)

As I ramp my load test it seems (1 or 2 times out of 10) the <domain object>.get(id) fails on the quartz thread.

In the [http-8080-1-thread] I can do <domain object>.get(id) which always works fine. So the save works.

So the question is... is there something I can do beyond the flush:true to make sure the object is immediately available on the other thread(s)??

Show
Martin Gee added a comment - I'm having a simular issue with Grails 1.3.5 (I've tried the toInteger - doesn't work)... Grails 1.3.5, MacOS, Java(TM) SE Runtime Environment (build 1.6.0_20-b02-279-10M3065) In load testing my app I'm having a random issue with <domain>.get(id) returning null. I create a domain object in a service and inturn (that same service) schedules a quartz job which immediately tries to read just newly created domain object (with a get(id) ). NOTE : these are 2 different threads... Flow: [http-8080-1-thread] <controller> -> my service -> create domain:save(flush:true), schedule quartz job [quartzScheduler_Worker-10-thread] <quartz> -> job -> get domain object (id in jobmap) As I ramp my load test it seems (1 or 2 times out of 10) the <domain object>.get(id) fails on the quartz thread. In the [http-8080-1-thread] I can do <domain object>.get(id) which always works fine. So the save works. So the question is... is there something I can do beyond the flush:true to make sure the object is immediately available on the other thread(s)??
Hide
Permalink
Graeme Rocher added a comment -

Attach an example that reproduces the issue in 1.3.5 then we can reopen the issue if it is a problem

Show
Graeme Rocher added a comment - Attach an example that reproduces the issue in 1.3.5 then we can reopen the issue if it is a problem
Hide
Permalink
Graeme Rocher added a comment -

Bulk closing bunch of resolved issues

Show
Graeme Rocher added a comment - Bulk closing bunch of resolved issues

People

Vote (7)
Watch (4)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.1#660-r161644) | Report a problem
Powered by a free Atlassian JIRA open source license for Grails project. Try JIRA - bug tracking software for your team.