Skip to content

Commit

Permalink
JRuby fix for concurrency issue with manager cache
Browse files Browse the repository at this point in the history
The configuration manager's alteration of its internal cache hash to
preserve previously dynamically defined values has been known to cause
problems with JRuby given its concurrent access to the hash object. We
have used `synchronize` and `Hash#dup` fixes previously, but still have
reports of issues.

When it comes down to it, the alteration of the hash was only added and
still only needed for use by the security agent. So let's just not even
attempt to alter the hash if JRuby is in play but the security agent is
not.
  • Loading branch information
fallwith committed Aug 8, 2024
1 parent 7dae0d9 commit dd9e115
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 0 deletions.
8 changes: 8 additions & 0 deletions lib/new_relic/agent/configuration/manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,14 @@ def reset_to_defaults
def reset_cache
return new_cache unless defined?(@cache) && @cache

# Modifying the @cache hash under JRuby - even with a `synchronize do`
# block and a `Hash#dup` operation - has been known to cause issues
# with JRuby for concurrent access of the hash while it is being
# modified. The hash really only needs to be modified for the benefit
# of the security agent, so if JRuby is in play and the security agent
# is not, don't attempt to modify the hash at all and return early.
return @cache if NewRelic::LanguageSupport.jruby? && !Agent.config[:'security.agent.enabled']

@lock.synchronize do
preserved = @cache.dup.select { |_k, v| DEPENDENCY_DETECTION_VALUES.include?(v) }
new_cache
Expand Down
13 changes: 13 additions & 0 deletions test/new_relic/agent/configuration/manager_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,19 @@ def test_logger_does_not_receive_excluded_settings
refute_includes(log, ':license_key')
end

def test_reset_cache_return_early_for_jruby
phony_cache = {dup_called: false}
def phony_cache.dup; self[:dup_called] = true; self; end
@manager.instance_variable_set(:@cache, phony_cache)
NewRelic::LanguageSupport.stub :jruby?, true do
@manager.reset_cache
end

refute phony_cache[:dup_called], 'Expected the use of JRuby to prevent the Hash#dup call!'
ensure
@manager.new_cache
end

private

def assert_parsed_labels(expected)
Expand Down

0 comments on commit dd9e115

Please sign in to comment.