Skip to content

Commit

Permalink
Fix memory leak on RDF::Util::Cache (#438)
Browse files Browse the repository at this point in the history
* Add cache tests
* Fix cache finalizers
  • Loading branch information
abrisse committed Apr 14, 2023
1 parent ac80b2f commit 58d8c52
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 1 deletion.
8 changes: 7 additions & 1 deletion lib/rdf/util/cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ def []=(key, value)
id = value.__id__
@cache[key] = id
@index[id] = key
ObjectSpace.define_finalizer(value, proc {|id| @cache.delete(@index.delete(id))})
ObjectSpace.define_finalizer(value, finalizer_proc)
end
value
end
Expand All @@ -100,6 +100,12 @@ def delete(key)
@cache.delete(key)
@index.delete(id) if id
end

private

def finalizer_proc
proc { |id| @cache.delete(@index.delete(id)) }
end
end # ObjectSpaceCache

##
Expand Down
80 changes: 80 additions & 0 deletions spec/util/cache_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
require_relative '../spec_helper'

describe RDF::Util::Cache do
subject(:cache) do
described_class.new(10)
end

describe '#capacity' do
it 'returns the cache size' do
expect(cache.capacity).to eq 10
end
end

describe '#size' do
it 'returns the cache size' do
cache[:key] = {}
expect(cache.size).to eq 1
end
end

describe '#[]' do
it 'returns the value' do
cache[:key] = {}
expect(cache[:key]).to eq({})
end
end

describe '#[]=' do
context 'when the cache is not full' do
it 'stores the value' do
expect {
cache[:key] = {}
}.to change(cache, :size).by(1)
end

it 'returns the value' do
expect(cache[:key] = {}).to eq({})
end
end

context 'when the cache is full' do
before do
10.times { |i| cache[i] = {} }
end

it 'does not store the value' do
expect {
cache[:key] = {}
}.not_to change(cache, :size)
end

it 'returns the value' do
expect(cache[:key] = {}).to eq({})
end
end
end

context 'when the GC starts' do
before do
100.times { |i| cache[i] = {}; nil }
end

# Sometimes the last reference is not gc
it 'cleans the unused references' do
expect {
GC.start
}.to change(cache, :size).by_at_most(-9)
end
end

describe '#delete' do
before do
cache[:key] = {}
end

it 'delete the value' do
expect { cache.delete(:key) }.to change(cache, :size).to(0)
end
end
end

0 comments on commit 58d8c52

Please sign in to comment.