Issue #4955 has been updated by Markus Roberts.
Actually, I've got two ideas of how to deal with the memory leak aspect: we
could either get rid of the hash and use the keys themselves as the
synchronization objects (by decorating them) which would look something like
(this is just a sketch):
<pre>
extend MonitorMixin
# Return the sync object associated with a given object.
def self.sync(x)
synchronize { x.extend MySyncModule unless x.is_a? MySyncModule; x }
end
</pre>
Alternatively we could make use of the fact that the sync objects are only used
to call synchronize like so:
<pre>
Puppet::Util.sync(file).synchronize(Sync::SH) do ...
</pre>
and replace the chained calls with something like:
<pre>
Puppet::Util.synchonize_on(file,Sync::SH) do ...
</pre>
which could be written (roughly):
<pre>
# Create a hash to store the different sync objects.
$syncresources = {}.extend MonitorMixin
def self.synchronize_on(x,type)
$syncresources.synchronize {
($syncresources[x] ||= [Sync.new,0]) += 1
}
$syncresources[resource].first.synchronize(type) { yield }
$syncresources.synchronize {
$syncresources.delete(x) unless ($syncresources[x][1] -= 1) > 0
}
end
</pre>
It's tempting to think we could dispense with the Sync objects altogether, but
that would mean duplicating the type logic :(
----------------------------------------
Bug #4955: Puppet::Util.sync is not thread safe
http://projects.puppetlabs.com/issues/4955
Author: Markus Roberts
Status: Accepted
Priority: High
Assignee: Markus Roberts
Category: threading
Target version:
Affected version: 0.24.0
Keywords:
Branch:
The routine for issuing reference specific sync objects is tricky to use
correctly since it is not thread safe (it also appears to leak memory):
<pre>
# Return the sync object associated with a given resource.
def self.sync(resource)
@@syncresources[resource] ||= Sync.new
@@syncresources[resource]
end
</pre>
The problem is somewhat muddled by the implication that the key object is a
resource (and thus may be thread bound) -- in fact many of the code paths use
something other than a resource as a key. This also affect threadlock, which
is built on top of sync.
Apparently I don't "get" irony, so I won't comment on that aspect of the
situation, but this is still something that ought to be fix (a "bug," to be
technical).
--
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.
