Skip to content

Commit

Permalink
Fix Relationship.instances cache for thread safety
Browse files Browse the repository at this point in the history
Taken from randym#477
  • Loading branch information
Kaizhi committed Feb 11, 2019
1 parent fba6756 commit 2989d91
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 23 deletions.
8 changes: 6 additions & 2 deletions lib/axlsx/package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand All @@ -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
Expand Down
50 changes: 29 additions & 21 deletions lib/axlsx/rels/relationship.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,43 +3,51 @@ module Axlsx
# A relationship defines a reference between package parts.
# @note Packages automatically manage relationships.
class Relationship

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.
# {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).
#
# Also, calling this avoids memory leaks (cached instances lingering around
# forever).
#
# Also, calling this avoids memory leaks (cached instances lingering around
# forever).
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.
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}.

# Generate and return a unique id (eg. `rId123`) Used for setting {#Id}.
#
# The generated id depends on the number of cached instances, so using
# {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

# The id of the relationship (eg. "rId123"). Most instances get their own unique id.
# The id of the relationship (eg. "rId123"). Most instances get their own unique id.
# However, some instances need to share the same id – see {#should_use_same_id_as?}
# for details.
# @return [String]
attr_reader :Id

# The location of the relationship target
# @return [String]
attr_reader :Target
Expand Down Expand Up @@ -69,8 +77,8 @@ def next_free_id
# The source object the relations belongs to (e.g. a hyperlink, drawing, ...). Needed when
# looking up the relationship for a specific object (see {Relationships#for}).
attr_reader :source_obj
# Initializes a new relationship.

# Initializes a new relationship.
# @param [Object] source_obj see {#source_obj}
# @param [String] type The type of the relationship
# @param [String] target The target for the relationship
Expand Down Expand Up @@ -105,12 +113,12 @@ def to_xml_string(str = '')
str << (h.map { |key, value| '' << key.to_s << '="' << Axlsx::coder.encode(value.to_s) << '"'}.join(' '))
str << '/>'
end

# Whether this relationship should use the same id as `other`.
#
# Instances designating the same relationship need to use the same id. We can not simply
# compare the {#Target} attribute, though: `foo/bar.xml`, `../foo/bar.xml`,
# `../../foo/bar.xml` etc. are all different but probably mean the same file (this
# compare the {#Target} attribute, though: `foo/bar.xml`, `../foo/bar.xml`,
# `../../foo/bar.xml` etc. are all different but probably mean the same file (this
# is especially an issue for relationships in the context of pivot tables). So lets
# just ignore this attribute for now (except when {#TargetMode} is set to `:External` –
# then {#Target} will be an absolute URL and thus can safely be compared).
Expand All @@ -124,6 +132,6 @@ def should_use_same_id_as?(other)
end
result
end

end
end

0 comments on commit 2989d91

Please sign in to comment.