From dd9e115038cc3f277b3136c90a7cf44c84ac3aea Mon Sep 17 00:00:00 2001 From: fallwith Date: Wed, 7 Aug 2024 23:40:36 -0700 Subject: [PATCH] JRuby fix for concurrency issue with manager cache 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. --- lib/new_relic/agent/configuration/manager.rb | 8 ++++++++ test/new_relic/agent/configuration/manager_test.rb | 13 +++++++++++++ 2 files changed, 21 insertions(+) diff --git a/lib/new_relic/agent/configuration/manager.rb b/lib/new_relic/agent/configuration/manager.rb index 0222501771..40249f8a8f 100644 --- a/lib/new_relic/agent/configuration/manager.rb +++ b/lib/new_relic/agent/configuration/manager.rb @@ -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 diff --git a/test/new_relic/agent/configuration/manager_test.rb b/test/new_relic/agent/configuration/manager_test.rb index ad3522294a..b574690a98 100644 --- a/test/new_relic/agent/configuration/manager_test.rb +++ b/test/new_relic/agent/configuration/manager_test.rb @@ -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)