diff --git a/lib/axlsx/package.rb b/lib/axlsx/package.rb index da661b34..2e9e14fc 100644 --- a/lib/axlsx/package.rb +++ b/lib/axlsx/package.rb @@ -100,11 +100,13 @@ def workbook=(workbook) DataTypeValidator.validate :Package_workbook, Workbook, # File.open('example_streamed.xlsx', 'w') { |f| f.write(s.read) } def serialize(output, confirm_valid=false) return false unless !confirm_valid || self.validate.empty? - Relationship.clear_cached_instances + Relationship.initialize_cached_instances Zip::OutputStream.open(output) do |zip| write_parts(zip) end true + ensure + Relationship.clear_cached_instances end @@ -113,11 +115,13 @@ def serialize(output, confirm_valid=false) # @return [StringIO|Boolean] False if confirm_valid and validation errors exist. rewound string IO if not. def to_stream(confirm_valid=false) return false unless !confirm_valid || self.validate.empty? - Relationship.clear_cached_instances + Relationship.initialize_cached_instances zip = write_parts(Zip::OutputStream.new(StringIO.new, true)) stream = zip.close_buffer stream.rewind stream + ensure + Relationship.clear_cached_instances end # Encrypt the package into a CFB using the password provided diff --git a/lib/axlsx/rels/relationship.rb b/lib/axlsx/rels/relationship.rb index 99dad367..66f53b2a 100644 --- a/lib/axlsx/rels/relationship.rb +++ b/lib/axlsx/rels/relationship.rb @@ -8,20 +8,28 @@ class << self # Keeps track of all instances of this class. # @return [Array] def instances - @instances ||= [] + Thread.current[:axlsx_relationship_cached_instances] ||= [] end - - # Clear cached instances. + + # Initialize cached instances. # # This should be called before serializing a package (see {Package#serialize} and # {Package#to_stream}) to make sure that serialization is idempotent (i.e. # Relationship instances are generated with the same IDs everytime the package # is serialized). + def initialize_cached_instances + Thread.current[:axlsx_relationship_cached_instances] = [] + end + + # Clear cached instances. + # + # This should be called after serializing a package (see {Package#serialize} and + # {Package#to_stream}) to free the memory allocated for cache. # # Also, calling this avoids memory leaks (cached instances lingering around # forever). def clear_cached_instances - @instances = [] + Thread.current[:axlsx_relationship_cached_instances] = nil end # Generate and return a unique id (eg. `rId123`) Used for setting {#Id}. @@ -30,7 +38,7 @@ def clear_cached_instances # {clear_cached_instances} will automatically reset the generated ids, too. # @return [String] def next_free_id - "rId#{@instances.size + 1}" + "rId#{instances.size + 1}" end end diff --git a/test/rels/tc_relationship.rb b/test/rels/tc_relationship.rb index add1654f..845f789e 100644 --- a/test/rels/tc_relationship.rb +++ b/test/rels/tc_relationship.rb @@ -13,6 +13,14 @@ def test_instances_with_same_attributes_share_id instance = Axlsx::Relationship.new(source_obj, Axlsx::WORKSHEET_R, 'target') assert_equal instance.Id, Axlsx::Relationship.new(source_obj, Axlsx::WORKSHEET_R, 'target').Id end + + def test_instances_cache_is_thread_safe + cache1, cache2 = nil + t1 = Thread.new { cache1 = Axlsx::Relationship.instances } + t2 = Thread.new { cache2 = Axlsx::Relationship.instances } + [t1, t2].each(&:join) + assert_not_same(cache1, cache2) + end def test_target_is_only_considered_for_same_attributes_check_if_target_mode_is_external source_obj = Object.new diff --git a/test/tc_package.rb b/test/tc_package.rb index d89f35b0..4bd0d429 100644 --- a/test/tc_package.rb +++ b/test/tc_package.rb @@ -236,6 +236,8 @@ def test_to_stream # this is just a roundabout guess for a package as it is build now # in testing. assert(stream.size > 80000) + # Cached instances should be cleared + assert(Axlsx::Relationship.instances.empty?) end def test_encrypt