diff --git a/CHANGELOG.md b/CHANGELOG.md index 05981c7713..e5dc1bf16e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## dev -Version updates framework detection and fixes Falcon dispatcher detection. +Version updates framework detection, fixes Falcon dispatcher detection, and addresses a JRuby specific concurrency issue. - **Feature: Improve framework detection accuracy for Grape and Padrino** @@ -12,6 +12,10 @@ Version updates framework detection and fixes Falcon dispatcher detection. Previously, we tried to use the object space to determine whether the [Falcon web server](https://github.com/socketry/falcon) was in use. However, Falcon is not added to the object space until after the environment report is generated, resulting in a `nil` dispatcher. Now, we revert to an earlier strategy that discovered the dispatcher using `File.basename`. Thank you, [@prateeksen](https://github.com/prateeksen) for reporting this issue and researching the problem. [Issue#2778](https://github.com/newrelic/newrelic-ruby-agent/issues/2778) [PR#2795](https://github.com/newrelic/newrelic-ruby-agent/pull/2795) +- **Bugfix: Address JRuby concurrency issue with config hash accessing** + + The agent's internal configuration class maintains a hash that occassionally gets rebuilt. During the rebuild, certain previously dynamically determined instrumentation values are preserved for the benefit of the [New Relic Ruby security agent](https://github.com/newrelic/csec-ruby-agent). After reports from JRuby customers regarding concurrency issues related to the hash being accessed while being modified, two separate fixes went into the hash rebuild logic previously: a `Hash#dup` operation and a `synchronize do` block. But errors were still reported. We ourselves remain unable to reproduce these concurrency errors despite using the same exact versions of JRuby and all reported software. After confirming that the hash access code in question is only needed for the Ruby security agent (which operates only in non-production dedicated security testing environments), we have introduced a new fix for JRuby customers that will simply skip over the troublesome code when JRuby is in play but the security agent is not. [PR#2798](https://github.com/newrelic/newrelic-ruby-agent/pull/2798) + ## v9.12.0 Version 9.12.0 adds support for the `newrelic_security` agent, introduces instrumentation for the LogStasher gem, improves instrumentation for the `redis-clustering` gem, and updates the Elasticsearch instrumentation to only attempt to get the cluster name once per client, even if it fails. 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)