Re: Deadlock issue

Hi Daniel,

On Apr 19, 2012, at 7:05 PM, Daniel Dekany wrote:

> Thursday, April 19, 2012, 12:32:19 PM, Jacopo Cappellato wrote:
> 
>> On Apr 19, 2012, at 11:45 AM, Daniel Dekany wrote:
>> 
>>> Then maybe you share the same
>>> freemarker.jar file (same full-path) for multiple applications, and
>>> are also using the deprecated default shared (static) ObjectWrapper
>>> instance (or some other ObjectWrapper instance which is somehow shared
>>> between applications). 
>> 
>> I think you are hitting the point Daniel; here is the OFBiz
>> framework code that performs the setup of the wrapper and freemarker
>> configuration; the same is then used by all webapplications to render templates:
>> 
>>    protected static BeansWrapper defaultOfbizWrapper =
>> BeansWrapper.getDefaultInstance();
> 
> Here, you should have `new BeansWrapper()`, assuming this runs once per
> webapp. Then the webapps will not share the introspection cache.
> 
> Now, maybe the OFBiz developers wanted the application to share the
> introspection cache for lower total RAM usage.

Yes, I think this was the main reason.

> However doing so also
> means more blocking (because access to the cache in synchronized),
> which I'm not sure how that affects you.

Yes, now I understand the cons of the approach we are following: we use less memory but the access is slower under high traffic because the cache is synchronized; I will discuss this in the OFBiz community.

> But of course the main
> problem here is the frequent cache flushing... One would think that
> adding an option for disabling reloading-detection would be
> sufficient, since BeansWrapper's introspection cache looks up classes
> by identity, not by name (so it could handle multiple versions of the
> same class). But the cache of ClassBasedModelFactory (used for statics
> and enums) uses the class name (a String) as the key, so... for now,
> the best would be just using `new BeansWrapper()`. (However, it should
> be checked if OFBiz uses BeansWrapper.getDefaultInstance() somewhere
> else too... certainly not, but better check it.)
> 
> I'm also surprised on the kind of wrapper used here. Most frameworks
> should just use `new DefaultObjectWrapper()`, or `bw = new
> BeansWrapper(); bw.setSimpleMapWrapper(true);`. However, to be honest,
> this is messy part of FreeMarker (which wrapper to use and why), so
> I'm not blaming the OFBiz developers. Do you access map items as
> `someMap.get(theKey)` in templates under OFBiz?

We do something like this:

${someMap.theKey}

Do you think we should use a different object wrapper?

Thanks,

Jacopo

> 
>>    protected static Configuration defaultOfbizConfig =
>> makeConfiguration(defaultOfbizWrapper);
>> 
>>    public static Configuration makeConfiguration(BeansWrapper wrapper) {
>>        Configuration newConfig = new Configuration();
>>        newConfig.setObjectWrapper(wrapper);
>>        newConfig.setSharedVariable("Static", wrapper.getStaticModels());
>>        newConfig.setLocalizedLookup(false);
>>        newConfig.setTemplateLoader(new FlexibleTemplateLoader());
>> 
>> newConfig.setAutoImports(UtilProperties.getProperties("freemarkerImports"));
>>        newConfig.setTemplateExceptionHandler(new
>> FreeMarkerWorker.OFBizTemplateExceptionHandler());
>>        // Transforms properties file set up as key=transform name, property=transform class name
>>        ClassLoader loader =
>> Thread.currentThread().getContextClassLoader();
>>        Enumeration<URL> resources;
>>        try {
>>            resources =
>> loader.getResources("freemarkerTransforms.properties");
>>        } catch (IOException e) {
>>            Debug.logError(e, "Could not load list of
>> freemarkerTransforms.properties", module);
>>            throw UtilMisc.initCause(new
>> InternalError(e.getMessage()), e);
>>        }
>>        while (resources.hasMoreElements()) {
>>            URL propertyURL = resources.nextElement();
>>            Debug.logInfo("loading properties: " + propertyURL, module);
>>            Properties props =
>> UtilProperties.getProperties(propertyURL);
>>            if (props == null || props.isEmpty()) {
>>                Debug.logError("Unable to locate properties file " + propertyURL, module);
>>            } else {
>>                loadTransforms(loader, props, newConfig);
>>            }
>>        }
>> 
>>        return newConfig;
>>    }
>> 
>> 
>> and in the same class we use this static method to render the template:
>> 
>>    public static Environment renderTemplate(Template template,
>> Map<String, Object> context, Appendable outWriter) throws TemplateException, IOException {
>>        // make sure there is no "null" string in there as FreeMarker will try to use it
>>        context.remove("null");
>>        // Since the template cache keeps a single instance of a
>> Template that is shared among users,
>>        // and since that Template instance is immutable, we need
>> to create an Environment instance and
>>        // use it to process the template with the user's settings.
>>        Environment env =
>> template.createProcessingEnvironment(context, (Writer) outWriter); 
>> // NOTE: this is where the process blocks in a deadlock
>>        applyUserSettings(env, context);
>>        env.process();   // NOTE: this is where the process blocks in a deadlock
>>        return env;
>>    }
>> 
>> Jacopo
> 
> -- 
> Best regards,
> Daniel Dekany
> 


------------------------------------------------------------------------------
For Developers, A Lot Can Happen In A Second.
Boundary is the first to Know...and Tell You.
Monitor Your Applications in Ultra-Fine Resolution. Try it FREE!
http://p.sf.net/sfu/Boundary-d2dvs2