Made fallback boot-time instead of runtime, remove unneeded functions

Author: kyewei

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;

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

lib/semian/sysv_state.rb

@@ -6,37 +6,25 @@ 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,
+                     :bind_initialize_memory_callback, :destroy
+      private :acquire_memory_object, :bind_initialize_memory_callback
 
       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

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
 

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

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

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
 

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

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