From 60d9687fff969249b4df69aa1559e0130815b7ca Mon Sep 17 00:00:00 2001 From: Kye Wei Date: Fri, 20 Nov 2015 20:52:19 +0000 Subject: [PATCH] Made fallback boot-time instead of runtime, remove unneeded functions --- ext/semian/semian_shared_memory_object.c | 47 ++++++++++-------------- lib/semian/sysv_shared_memory.rb | 39 ++++---------------- lib/semian/sysv_state.rb | 24 ++++-------- test/circuit_breaker_test.rb | 1 + test/resource_test.rb | 28 +++++++++----- test/simple_integer_test.rb | 4 +- test/simple_sliding_window_test.rb | 4 +- test/simple_state_test.rb | 4 +- test/sysv_integer_test.rb | 14 +++---- test/sysv_sliding_window_test.rb | 21 ++++++----- test/sysv_state_test.rb | 6 +-- 11 files changed, 81 insertions(+), 111 deletions(-) diff --git a/ext/semian/semian_shared_memory_object.c b/ext/semian/semian_shared_memory_object.c index d397f6c16..65f20d93a 100644 --- a/ext/semian/semian_shared_memory_object.c +++ b/ext/semian/semian_shared_memory_object.c @@ -82,25 +82,29 @@ semian_shm_object_sizeof(VALUE klass, VALUE type) return INT2NUM(sizeof(long)); // Can definitely add more else - return INT2NUM(0); + rb_raise(rb_eTypeError, "%s is not a valid C type", rb_id2name(SYM2ID(type))); } VALUE -semian_shm_object_acquire(VALUE self, VALUE name, VALUE byte_size, VALUE permissions) +semian_shm_object_acquire(VALUE self, VALUE name, VALUE data_layout, VALUE permissions) { semian_shm_object *ptr; TypedData_Get_Struct(self, semian_shm_object, &semian_shm_object_type, ptr); if (TYPE(name) != T_SYMBOL && TYPE(name) != T_STRING) rb_raise(rb_eTypeError, "id must be a symbol or string"); - if (TYPE(byte_size) != T_FIXNUM) - rb_raise(rb_eTypeError, "expected integer for byte_size"); + if (TYPE(data_layout) != T_ARRAY) + rb_raise(rb_eTypeError, "expected array for data_layout"); if (TYPE(permissions) != T_FIXNUM) rb_raise(rb_eTypeError, "expected integer for permissions"); - if (NUM2SIZET(byte_size) <= 0) - rb_raise(rb_eArgError, "byte_size must be larger than 0"); + int byte_size = 0; + for (int i = 0; i < RARRAY_LEN(data_layout); ++i) + byte_size += NUM2INT(semian_shm_object_sizeof(rb_cObject, RARRAY_PTR(data_layout)[i])); + + if (byte_size <= 0) + rb_raise(rb_eArgError, "total size must be larger than 0"); const char *id_str = NULL; if (TYPE(name) == T_SYMBOL) { @@ -109,7 +113,7 @@ semian_shm_object_acquire(VALUE self, VALUE name, VALUE byte_size, VALUE permiss id_str = RSTRING_PTR(name); } ptr->key = generate_key(id_str); - ptr->byte_size = NUM2SIZET(byte_size); // byte_size >=1 or error would have been raised earlier + ptr->byte_size = byte_size; // byte_size >=1 or error would have been raised earlier ptr->semid = -1; // id's default to -1 ptr->shmid = -1; ptr->shm_address = 0; // address defaults to NULL @@ -124,7 +128,9 @@ semian_shm_object_acquire(VALUE self, VALUE name, VALUE byte_size, VALUE permiss semian_shm_object_acquire_semaphore(self); semian_shm_object_synchronize(self); - return self; + + + return Qtrue; } VALUE @@ -349,13 +355,10 @@ semian_shm_object_synchronize_memory_and_size(VALUE self, VALUE is_master_obj) { } else { void *old_shm_address = ptr->shm_address; size_t old_byte_size = ptr->byte_size; - void *old_memory_content = NULL; + unsigned char old_memory_content[old_byte_size]; - char old_memory_content_tmp[old_byte_size]; - memcpy(old_memory_content_tmp, old_shm_address, old_byte_size); + memcpy(old_memory_content, old_shm_address, old_byte_size); semian_shm_object_cleanup_memory(self); - old_memory_content = malloc(old_byte_size); - memcpy(old_memory_content, old_memory_content_tmp, old_byte_size); if (-1 == (ptr->shmid = shmget(key, requested_byte_size, IPC_CREAT | IPC_EXCL | ptr->permissions))) { rb_raise(eSyscall, "shmget failed to create new resized memory with key %d shmid %d errno %d (%s)", key, ptr->shmid, errno, strerror(errno)); @@ -366,7 +369,6 @@ semian_shm_object_synchronize_memory_and_size(VALUE self, VALUE is_master_obj) { ptr->byte_size = requested_byte_size; ptr->initialize_memory(ptr->byte_size, ptr->shm_address, old_memory_content, old_byte_size); - free(old_memory_content); } } return self; @@ -419,30 +421,19 @@ semian_shm_object_shmid(VALUE self) return INT2NUM(ptr->shmid); } -static VALUE -semian_shm_object_byte_size(VALUE self) -{ - semian_shm_object *ptr; - TypedData_Get_Struct(self, semian_shm_object, &semian_shm_object_type, ptr); - semian_shm_object_synchronize(self); - return INT2NUM(ptr->byte_size); -} - void Init_semian_shm_object (void) { VALUE cSemianModule = rb_const_get(rb_cObject, rb_intern("Semian")); VALUE cSysVSharedMemory = rb_const_get(cSemianModule, rb_intern("SysVSharedMemory")); - rb_define_method(cSysVSharedMemory, "_acquire", semian_shm_object_acquire, 3); - rb_define_method(cSysVSharedMemory, "_destroy", semian_shm_object_destroy, 0); - rb_define_method(cSysVSharedMemory, "_synchronize", semian_shm_object_synchronize, 0); + rb_define_method(cSysVSharedMemory, "acquire_memory_object", semian_shm_object_acquire, 3); + rb_define_method(cSysVSharedMemory, "destroy", semian_shm_object_destroy, 0); + rb_define_method(cSysVSharedMemory, "synchronize", semian_shm_object_synchronize, 0); rb_define_method(cSysVSharedMemory, "semid", semian_shm_object_semid, 0); rb_define_method(cSysVSharedMemory, "shmid", semian_shm_object_shmid, 0); - rb_define_method(cSysVSharedMemory, "byte_size", semian_shm_object_byte_size, 0); - rb_define_singleton_method(cSysVSharedMemory, "_sizeof", semian_shm_object_sizeof, 1); rb_define_singleton_method(cSysVSharedMemory, "replace_alloc", semian_shm_object_replace_alloc, 1); decrement.sem_num = kSHMIndexTicketLock; diff --git a/lib/semian/sysv_shared_memory.rb b/lib/semian/sysv_shared_memory.rb index 8f3b92838..58abe7ee2 100644 --- a/lib/semian/sysv_shared_memory.rb +++ b/lib/semian/sysv_shared_memory.rb @@ -1,12 +1,5 @@ module Semian module SysVSharedMemory #:nodoc: - @type_size = {} - def self.sizeof(type) - size = (@type_size[type.to_sym] ||= (respond_to?(:_sizeof) ? _sizeof(type.to_sym) : 0)) - raise TypeError.new("#{type} is not a valid C type") if size <= 0 - size - end - def self.included(base) def base.do_with_sync(*names) names.each do |name| @@ -30,38 +23,20 @@ def shmid -1 end - def synchronize(&block) - if respond_to?(:_synchronize) && @using_shared_memory - _synchronize(&block) - else - yield if block_given? - end + def synchronize + yield if block_given? end - alias_method :transaction, :synchronize - def destroy - if respond_to?(:_destroy) && @using_shared_memory - _destroy - else - super - end + super end private - def shared? - @using_shared_memory - end - - def acquire_memory_object(name, data_layout, permissions) - return @using_shared_memory = false unless Semian.semaphores_enabled? && respond_to?(:_acquire) - - byte_size = data_layout.inject(0) { |sum, type| sum + ::Semian::SysVSharedMemory.sizeof(type) } - raise TypeError.new("Given data layout is 0 bytes: #{data_layout.inspect}") if byte_size <= 0 - # Calls C layer to acquire/create a memory block, calling #initialize_memory in the process, see below - _acquire(name, byte_size, permissions) - @using_shared_memory = true + def acquire_memory_object(*) + # Concrete classes must call this method before accessing shared memory + # If SysV is enabled, a C method overrides this stub and returns true if acquiring succeeds + false end def bind_initialize_memory_callback diff --git a/lib/semian/sysv_state.rb b/lib/semian/sysv_state.rb index 0312a4267..1aabaac13 100644 --- a/lib/semian/sysv_state.rb +++ b/lib/semian/sysv_state.rb @@ -6,37 +6,29 @@ class State < Semian::Simple::State #:nodoc: include SysVSharedMemory extend Forwardable - def_delegators :@integer, :semid, :shmid, :synchronize, :transaction, - :shared?, :acquire_memory_object, :initialize_memory - private :shared?, :acquire_memory_object, :initialize_memory + SYM_TO_NUM = {closed: 0, open: 1, half_open: 2}.freeze + NUM_TO_SYM = SYM_TO_NUM.invert.freeze + + def_delegators :@integer, :semid, :shmid, :synchronize, + :acquire_memory_object, :initialize_memory + private :acquire_memory_object, :initialize_memory def initialize(name:, permissions:) @integer = Semian::SysV::Integer.new(name: name, permissions: permissions) - initialize_lookup end def destroy - super @integer.destroy end def value - @num_to_sym.fetch(@integer.value) { raise ArgumentError } + NUM_TO_SYM.fetch(@integer.value) { raise ArgumentError } end private def value=(sym) - @integer.value = @sym_to_num.fetch(sym) { raise ArgumentError } - end - - def initialize_lookup - # Assume symbol_list[0] is mapped to 0 - # Cannot just use #object_id since #object_id for symbols is different in every run - # For now, implement a C-style enum type backed by integers - - @sym_to_num = {closed: 0, open: 1, half_open: 2} - @num_to_sym = @sym_to_num.invert + @integer.value = SYM_TO_NUM.fetch(sym) { raise ArgumentError } end end end diff --git a/test/circuit_breaker_test.rb b/test/circuit_breaker_test.rb index 37abf78d6..441a2e88a 100644 --- a/test/circuit_breaker_test.rb +++ b/test/circuit_breaker_test.rb @@ -140,6 +140,7 @@ def test_shared_fresh_worker_killed_should_not_reset_circuit_breaker_data reader.gets Semian.register(:unique_res, tickets: 1, exceptions: [SomeError], error_threshold: 2, error_timeout: 5, success_threshold: 1) Process.kill(9, pid) + Process.waitall assert_circuit_opened Semian[:unique_res] end diff --git a/test/resource_test.rb b/test/resource_test.rb index fa48c6063..f9fd2cf0c 100644 --- a/test/resource_test.rb +++ b/test/resource_test.rb @@ -108,19 +108,29 @@ def test_acquire_timeout_override end def test_acquire_with_fork - resource = create_resource :testing, tickets: 2, timeout: 0.5 - - resource.acquire do - fork do - resource.acquire do - assert_raises Semian::TimeoutError do - resource.acquire {} - end + pids = [] + + 2.times do + reader, writer = IO.pipe + pids << fork do + create_resource(:testing, tickets: 2, timeout: 0.5).acquire do + reader.close + writer.puts "Acquired" + writer.close + sleep end end + reader.gets + end + + assert_raises Semian::TimeoutError do + create_resource(:testing, tickets: 2, timeout: 0.5).acquire {} + end - Process.wait + pids.each do |pid| + Process.kill(9, pid) end + Process.waitall end def test_acquire_releases_on_kill diff --git a/test/simple_integer_test.rb b/test/simple_integer_test.rb index 195051d9f..5e52c1949 100644 --- a/test/simple_integer_test.rb +++ b/test/simple_integer_test.rb @@ -1,10 +1,10 @@ require 'test_helper' class TestSimpleInteger < MiniTest::Unit::TestCase - CLASS = ::Semian::Simple::Integer + KLASS = ::Semian::Simple::Integer def setup - @integer = CLASS.new + @integer = KLASS.new end def teardown diff --git a/test/simple_sliding_window_test.rb b/test/simple_sliding_window_test.rb index 0e673e368..114871805 100644 --- a/test/simple_sliding_window_test.rb +++ b/test/simple_sliding_window_test.rb @@ -1,10 +1,10 @@ require 'test_helper' class TestSimpleSlidingWindow < MiniTest::Unit::TestCase - CLASS = ::Semian::Simple::SlidingWindow + KLASS = ::Semian::Simple::SlidingWindow def setup - @sliding_window = CLASS.new(max_size: 6) + @sliding_window = KLASS.new(max_size: 6) @sliding_window.clear end diff --git a/test/simple_state_test.rb b/test/simple_state_test.rb index edab7c227..f05d86a27 100644 --- a/test/simple_state_test.rb +++ b/test/simple_state_test.rb @@ -1,10 +1,10 @@ require 'test_helper' class TestSimpleState < MiniTest::Unit::TestCase - CLASS = ::Semian::Simple::State + KLASS = ::Semian::Simple::State def setup - @state = CLASS.new + @state = KLASS.new end def teardown diff --git a/test/sysv_integer_test.rb b/test/sysv_integer_test.rb index 45149e540..1d140cb40 100644 --- a/test/sysv_integer_test.rb +++ b/test/sysv_integer_test.rb @@ -1,10 +1,10 @@ require 'test_helper' class TestSysVInteger < MiniTest::Unit::TestCase - CLASS = ::Semian::SysV::Integer + KLASS = ::Semian::SysV::Integer def setup - @integer = CLASS.new(name: 'TestSysVInteger', permissions: 0660) + @integer = KLASS.new(name: 'TestSysVInteger', permissions: 0660) @integer.reset end @@ -15,7 +15,7 @@ def teardown include TestSimpleInteger::IntegerTestCases def test_memory_is_shared - integer_2 = CLASS.new(name: 'TestSysVInteger', permissions: 0660) + integer_2 = KLASS.new(name: 'TestSysVInteger', permissions: 0660) integer_2.value = 100 assert_equal 100, @integer.value @integer.value = 200 @@ -26,13 +26,13 @@ def test_memory_is_shared def test_memory_not_reset_when_at_least_one_worker_using_it @integer.value = 109 - integer_2 = CLASS.new(name: 'TestSysVInteger', permissions: 0660) + integer_2 = KLASS.new(name: 'TestSysVInteger', permissions: 0660) assert_equal @integer.value, integer_2.value reader, writer = IO.pipe pid = fork do reader.close - integer_3 = CLASS.new(name: 'TestSysVInteger', permissions: 0660) + integer_3 = KLASS.new(name: 'TestSysVInteger', permissions: 0660) assert_equal 109, integer_3.value integer_3.value = 110 writer.puts "Done" @@ -47,11 +47,11 @@ def test_memory_not_reset_when_at_least_one_worker_using_it def test_memory_reset_when_no_workers_using_it fork do - integer = CLASS.new(name: 'TestSysVInteger_2', permissions: 0660) + integer = KLASS.new(name: 'TestSysVInteger_2', permissions: 0660) integer.value = 109 end Process.waitall - @integer = CLASS.new(name: 'TestSysVInteger_2', permissions: 0660) + @integer = KLASS.new(name: 'TestSysVInteger_2', permissions: 0660) assert_equal 0, @integer.value end end diff --git a/test/sysv_sliding_window_test.rb b/test/sysv_sliding_window_test.rb index d18f600f7..aca436797 100644 --- a/test/sysv_sliding_window_test.rb +++ b/test/sysv_sliding_window_test.rb @@ -1,10 +1,10 @@ require 'test_helper' class TestSysVSlidingWindow < MiniTest::Unit::TestCase - CLASS = ::Semian::SysV::SlidingWindow + KLASS = ::Semian::SysV::SlidingWindow def setup - @sliding_window = CLASS.new(max_size: 6, + @sliding_window = KLASS.new(max_size: 6, name: 'TestSysVSlidingWindow', permissions: 0660) @sliding_window.clear @@ -25,7 +25,7 @@ def test_forcefully_killing_worker_holding_on_to_semaphore_releases_it reader, writer = IO.pipe pid = fork do reader.close - sliding_window_2 = CLASS.new(max_size: 6, + sliding_window_2 = KLASS.new(max_size: 6, name: 'TestSysVSlidingWindow', permissions: 0660) sliding_window_2.synchronize do @@ -37,6 +37,7 @@ def test_forcefully_killing_worker_holding_on_to_semaphore_releases_it reader.gets Process.kill(9, pid) + Process.waitall Timeout.timeout(1) do # assure dont hang @sliding_window << 100 @@ -46,7 +47,7 @@ def test_forcefully_killing_worker_holding_on_to_semaphore_releases_it def test_sliding_window_memory_is_actually_shared assert_equal 0, @sliding_window.size - sliding_window_2 = CLASS.new(max_size: 6, + sliding_window_2 = KLASS.new(max_size: 6, name: 'TestSysVSlidingWindow', permissions: 0660) assert_equal 0, sliding_window_2.size @@ -66,7 +67,7 @@ def test_sliding_window_memory_is_actually_shared def test_restarting_worker_should_not_reset_queue @sliding_window << 10 << 20 << 30 - sliding_window_2 = CLASS.new(max_size: 6, + sliding_window_2 = KLASS.new(max_size: 6, name: 'TestSysVSlidingWindow', permissions: 0660) assert_sliding_window(sliding_window_2, [10, 20, 30], 6) @@ -74,7 +75,7 @@ def test_restarting_worker_should_not_reset_queue assert_sliding_window(sliding_window_2, [10, 20], 6) assert_sliding_windows_in_sync(@sliding_window, sliding_window_2) - sliding_window_3 = CLASS.new(max_size: 6, + sliding_window_3 = KLASS.new(max_size: 6, name: 'TestSysVSlidingWindow', permissions: 0660) assert_sliding_window(sliding_window_3, [10, 20], 6) @@ -87,7 +88,7 @@ def test_other_workers_automatically_switching_to_new_memory_resizing_up_or_down # Test explicit resizing, and resizing through making new memory associations # B resize down through init - sliding_window_2 = CLASS.new(max_size: 4, + sliding_window_2 = KLASS.new(max_size: 4, name: 'TestSysVSlidingWindow', permissions: 0660) sliding_window_2 << 80 << 90 << 100 << 110 << 120 @@ -100,7 +101,7 @@ def test_other_workers_automatically_switching_to_new_memory_resizing_up_or_down assert_sliding_window(@sliding_window, [110, 120], 2) # B resize up through init - sliding_window_2 = CLASS.new(max_size: 4, + sliding_window_2 = KLASS.new(max_size: 4, name: 'TestSysVSlidingWindow', permissions: 0660) assert_sliding_windows_in_sync(@sliding_window, sliding_window_2) @@ -113,7 +114,7 @@ def test_other_workers_automatically_switching_to_new_memory_resizing_up_or_down assert_sliding_window(@sliding_window, [110, 120, 130], 6) # B resize down through init - sliding_window_2 = CLASS.new(max_size: 2, + sliding_window_2 = KLASS.new(max_size: 2, name: 'TestSysVSlidingWindow', permissions: 0660) assert_sliding_windows_in_sync(@sliding_window, sliding_window_2) @@ -125,7 +126,7 @@ def test_other_workers_automatically_switching_to_new_memory_resizing_up_or_down assert_sliding_window(@sliding_window, [120, 130], 4) # B resize up through init - sliding_window_2 = CLASS.new(max_size: 6, + sliding_window_2 = KLASS.new(max_size: 6, name: 'TestSysVSlidingWindow', permissions: 0660) assert_sliding_windows_in_sync(@sliding_window, sliding_window_2) diff --git a/test/sysv_state_test.rb b/test/sysv_state_test.rb index 768602a23..112196266 100644 --- a/test/sysv_state_test.rb +++ b/test/sysv_state_test.rb @@ -1,10 +1,10 @@ require 'test_helper' class TestSysVState < MiniTest::Unit::TestCase - CLASS = ::Semian::SysV::State + KLASS = ::Semian::SysV::State def setup - @state = CLASS.new(name: 'TestSysVState', + @state = KLASS.new(name: 'TestSysVState', permissions: 0660) @state.reset end @@ -19,7 +19,7 @@ def test_memory_is_shared assert_equal :closed, @state.value @state.open - state_2 = CLASS.new(name: 'TestSysVState', + state_2 = KLASS.new(name: 'TestSysVState', permissions: 0660) assert_equal :open, state_2.value assert state_2.open?