Issue #4244 has been updated by Markus Roberts.

Status changed from Investigating to Accepted

So this appears to have com in with:

<pre>
commit 5f1c228767432591a7e4b77666e9b1c7d70a095c
Author: Luke Kanies <[email protected]>
Date:   Sun May 17 16:54:06 2009 -0500

    Adding caching to the Environment class
</pre>

What confuses me is that I thought we'd addressed this in:

<pre>
commit bea561fd90bbc459fa0157b11455be8e4e8c39d6
Author: Markus Roberts <[email protected]>
Date:   Thu Jun 10 22:51:12 2010 -0700

    Part 2 of fix for #1175 (functions in environments)
    
    Jesse and I are shooting for the minimal viable fix here, with the idea that
    a great deal of refactoring is needed but isn't appropriate at this time.  
The
    changes in this commit are:
    
    [snip]

    * Turn off caching of the modulepath since it potentially changes for each 
node
</pre>

and

<pre>
commit 43ec46b2cca783a9f08599c70e87d4d8d5ade340
Author: Markus Roberts <[email protected]>
Date:   Mon Jun 14 18:22:40 2010 -0700

    Improving fix for #1175; tightening thread safety

</pre>

from MarkusQ:ticket/master/1175-part2 but the code in master does not match my 
recollection.  I also recall discussing this with Jesse and Luke specifically 
objecting to the removal of the caching.


----------------------------------------
Bug #4244: Multithread issue with cached attributes
http://projects.puppetlabs.com/issues/4244

Author: Brice Figureau
Status: Accepted
Priority: Normal
Assigned to: 
Category: threading
Target version: 
Affected version: 2.6.0rc3
Keywords: 
Branch: 


The definition for cached attributes (see puppet/util/cacher.rb) uses a hash 
and a construction which is not thread safe.

This one was found under JRuby with the following stack trace:
<pre>
/home/brice/devl/puppet/lib/puppet/util/autoload.rb:134:in `module_directories'
/home/brice/devl/puppet/lib/puppet/util/autoload.rb:142:in `search_directories'
/home/brice/devl/puppet/lib/puppet/util/autoload.rb:127:in `searchpath'
/home/brice/devl/puppet/lib/puppet/util/autoload.rb:75:in `load'
/home/brice/devl/puppet/lib/puppet/metatype/manager.rb:124:in `type'
/home/brice/devl/puppet/lib/puppet/resource.rb:360:in 
`find_builtin_resource_type'
/home/brice/devl/puppet/lib/puppet/resource.rb:356:in `find_resource_type'
/home/brice/devl/puppet/lib/puppet/resource.rb:442:in `resolve_type'
/home/brice/devl/puppet/lib/puppet/resource.rb:427:in `resolve_type_and_title'
/home/brice/devl/puppet/lib/puppet/resource.rb:179:in `initialize'
/home/brice/devl/puppet/lib/puppet/parser/resource.rb:109:in `initialize'
/home/brice/devl/puppet/lib/puppet/parser/ast/resource.rb:39:in `new'
/home/brice/devl/puppet/lib/puppet/parser/ast/resource.rb:39:in `evaluate'
/home/brice/devl/puppet/lib/puppet/util/errors.rb:35:in `exceptwrap'
/home/brice/devl/puppet/lib/puppet/parser/ast/resource.rb:37:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast/resource.rb:36:in `collect'
/home/brice/devl/puppet/lib/puppet/parser/ast/resource.rb:36:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast.rb:72:in `safeevaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:35:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:34:in `collect'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:34:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast.rb:72:in `safeevaluate'
/home/brice/devl/puppet/lib/puppet/resource/type.rb:75:in `evaluate_code'
/home/brice/devl/puppet/lib/puppet/parser/resource.rb:72:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:150:in `evaluate_classes'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:141:in `each'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:141:in `evaluate_classes'
/home/brice/devl/puppet/lib/puppet/parser/functions/include.rb:6:in 
`function_include'
/home/brice/devl/puppet/lib/puppet/parser/ast/function.rb:34:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast.rb:72:in `safeevaluate'
/home/brice/devl/puppet/lib/puppet/resource/type.rb:75:in `evaluate_code'
/home/brice/devl/puppet/lib/puppet/parser/resource.rb:72:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:150:in `evaluate_classes'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:141:in `each'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:141:in `evaluate_classes'
/home/brice/devl/puppet/lib/puppet/parser/functions/include.rb:6:in 
`function_include'
/home/brice/devl/puppet/lib/puppet/parser/ast/function.rb:34:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast.rb:72:in `safeevaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:35:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:34:in `collect'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:34:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast.rb:72:in `safeevaluate'
/home/brice/devl/puppet/lib/puppet/resource/type.rb:75:in `evaluate_code'
/home/brice/devl/puppet/lib/puppet/parser/resource.rb:72:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:150:in `evaluate_classes'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:141:in `each'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:141:in `evaluate_classes'
/home/brice/devl/puppet/lib/puppet/parser/functions/include.rb:6:in 
`function_include'
/home/brice/devl/puppet/lib/puppet/parser/ast/function.rb:34:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast.rb:72:in `safeevaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:35:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:34:in `collect'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:34:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast.rb:72:in `safeevaluate'
/home/brice/devl/puppet/lib/puppet/resource/type.rb:75:in `evaluate_code'
/home/brice/devl/puppet/lib/puppet/parser/resource.rb:72:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:150:in `evaluate_classes'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:141:in `each'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:141:in `evaluate_classes'
/home/brice/devl/puppet/lib/puppet/parser/functions/include.rb:6:in 
`function_include'
/home/brice/devl/puppet/lib/puppet/parser/ast/function.rb:34:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast.rb:72:in `safeevaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:35:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:34:in `collect'
/home/brice/devl/puppet/lib/puppet/parser/ast/astarray.rb:34:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/ast.rb:72:in `safeevaluate'
/home/brice/devl/puppet/lib/puppet/resource/type.rb:75:in `evaluate_code'
/home/brice/devl/puppet/lib/puppet/parser/resource.rb:72:in `evaluate'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:224:in `evaluate_ast_node'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:102:in `compile'
/home/brice/devl/puppet/lib/puppet/parser/compiler.rb:18:in `compile'
/home/brice/devl/puppet/lib/puppet/indirector/catalog/compiler.rb:77:in 
`compile'
/home/brice/devl/puppet/lib/puppet/util.rb:175:in `benchmark'
/home/brice/devl/jruby/lib/ruby/1.8/benchmark.rb:308:in `realtime'
/home/brice/devl/puppet/lib/puppet/util.rb:174:in `benchmark'
/home/brice/devl/puppet/lib/puppet/indirector/catalog/compiler.rb:75:in 
`compile'
/home/brice/devl/puppet/lib/puppet/indirector/catalog/compiler.rb:34:in `find'
/home/brice/devl/puppet/lib/puppet/indirector/indirection.rb:193:in `find'
/home/brice/devl/puppet/lib/puppet/indirector.rb:50:in `find'
/home/brice/devl/puppet/lib/puppet/network/http/handler.rb:101:in `do_find'
/home/brice/devl/puppet/lib/puppet/network/http/handler.rb:68:in `process'
/home/brice/devl/puppet/lib/puppet/network/http/rack.rb:48:in `call'
file:/home/brice/devl/puppet/../jetty-rackup/jars/jruby-rack-0.9.6.jar!/rack/handler/servlet.rb:19:in
 `call'
:1
err: Puppet::Parser::AST::Resource failed with error NoMethodError: undefined 
method `collect' for nil:NilClass at 
/home/brice/cvs/puppet/modules/apache/manifests/init.pp:239 on node 
corp1.daysofwonder.com
</pre>

The cached_value method should be protected by a critical section, as it sets 
the shared value, and at the same time can remove it (if expired). There is 
also a possibility for the variable to be initialized twice or the value to be 
replaced if assignement is not atomic.



-- 
You have received this notification because you have either subscribed to it, 
or are involved in it.
To change your notification preferences, please click here: 
http://projects.puppetlabs.com/my/account

-- 
You received this message because you are subscribed to the Google Groups 
"Puppet Bugs" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to 
[email protected].
For more options, visit this group at 
http://groups.google.com/group/puppet-bugs?hl=en.

Reply via email to