thread safety issues in facter?
Ken Barber
https://github.com/mkincaid/facter/blob/ticket/2847/lib/facter/mounts.rb#L69-71
Which is for ticket:
http://projects.puppetlabs.com/issues/2847
When talking to KW he said he ran into thread safety issues, hence why
he used a mutex. Has anyone seen this before? Its a concern because we
should be fixing cause not symptom ...
There is some smell of it here:
http://projects.puppetlabs.com/issues/4246
But I'm not sure. Has anyone else seen this? I'm asking KW to
replicate this with the latest facter regardless.
ken.
Daniel Pittman
> So I was looking at a patch from mkincaid & Krzysztof Wilczynski
> https://github.com/mkincaid/facter/blob/ticket/2847/lib/facter/mounts.rb#L69-71
>
> Which is for ticket:
> http://projects.puppetlabs.com/issues/2847
>
> When talking to KW he said he ran into thread safety issues, hence why
> he used a mutex. Has anyone seen this before? Its a concern because we
> should be fixing cause not symptom ...
Generally, I think anything that we accept into core should be written
to be thread safe by not touching any shared resources, rather than by
locking around shared structures...
...ah, and that is why they have trouble. They are trying to
statically discover the list of data, then bind facts to those values
once, rather than trusting to the cache layer in facter.
They should pull all the logic into the setcode block, or a helper
called from it, and accept perhaps calculating it twice.
> There is some smell of it here:
> http://projects.puppetlabs.com/issues/4246
Very different issue: since we do actually thread our master, we need
the *infrastructure* around Facter to be thread-safe. Which is: in
our code loading facts we need to be thread-safe, but for writing a
thread you shouldn't need to care.
Also, you won't be able to reproduce that specific failure without
JRuby. Everything else has pretend threads and would make it stupidly
harder to hit this failure mode. (...or is passenger, which isn't
threaded.)
Daniel
--
⎋ Puppet Labs Developer – http://puppetlabs.com
♲ Made with 100 percent post-consumer electrons
markus
> When talking to KW he said he ran into thread safety issues, hence why
> he used a mutex. Has anyone seen this before? Its a concern because we
> should be fixing cause not symptom ...
Brice and I did a fair amount of thread safety cleanup in core at one
point, but I don't recall much (if any) progress being made on the
corresponding issues in facter (where they were less salient).
The issues we encountered were:
* Code that just wasn't thread safe
* Code that was green thread safe but not fiber/true thread safe
* Code that was thread safe on the ruby side but FFIed out to code
that wasn't or (as with the SSL library) made different assumptions.
Brice is generally a great source of insight on questions like this.
As for why you might handle something like this in the leaves rather
than the framework (note: I have not looked into the specific issue at
hand) it can often lead to what amounts to a global lock, obviating any
advantage you might expect from concurrency. Making a framework that is
"thread safe" regardless what client-code-fragments which have access to
shared state do ultimately tends towards "just run them one at a time in
a single thread" which sort of defeats the purpose.
-- M
R.I.Pienaar
----- Original Message -----
>
> As for why you might handle something like this in the leaves rather
> than the framework (note: I have not looked into the specific issue
> at hand) it can often lead to what amounts to a global lock, obviating
> any advantage you might expect from concurrency. Making a framework that
> is "thread safe" regardless what client-code-fragments which have access
> to shared state do ultimately tends towards "just run them one at a time
> in a single thread" which sort of defeats the purpose.
for facter I'd think this is totally the right way to go. Need to consider
the audience, expecting our users to do locking and semaphores is just crazyness
the Facter framework should just make it safe, the user experience should be
that simple code behaves well and can add a fact.
I recall at least one instance of a fact that did its own locking caused me
frequent deadlock issues when using facter as a lib in other code, once I
ditched that fact the problem went away.
markus
> ...ah, and that is why they have trouble. They are trying to
> statically discover the list of data, then bind facts to those values
> once, rather than trusting to the cache layer in facter.
>
> They should pull all the logic into the setcode block, or a helper
> called from it, and accept perhaps calculating it twice.
Bingo! +1
-- M
Brice Figureau
I used to be able to replicate #4246 quite easily with puppet-load on
the first run when several threads enters facter at the same time, and
they all try to resolve facts. It wasn't even a problem specific to
jruby, I was able to trigger it with MRI 1.8.7.
--
Brice Figureau
My Blog: http://www.masterzen.fr/
Krzysztof Wilczynski
[...]
> > called from it, and accept perhaps calculating it twice.
>
> Bingo! +1
within the "setcode" block and/or use helper when needed (i.e.
yielding more than one value, etc) :-)
This is also to make these facts fit nicely within Facter 1.7 caching
framework (alongside removing mutexes, etc).
KW