Merge pull request #52 from chef/lcg/dlopen-extension

add DLopen extension

12 files changed, 298 insertions, 100 deletions

diff --git a/Gemfile b/Gemfile
index 45b8081..34b35ca 100644
--- a/Gemfile
+++ b/Gemfile
@@ -6,6 +6,11 @@ platforms :rbx do
   gem 'rubysl', '~> 2.0'
 end
 
+group :development do
+  # for testing loading concurrently with yajl-ruby, not on jruby
+  gem 'yajl-ruby', :platforms => [ :ruby, :mswin, :mingw ]
+end
+
 group :development_extras do
   gem 'rubocop', '= 0.21.0'
   gem 'reek', '= 1.3.7'
diff --git a/README.md b/README.md
index de23bfc..2458331 100644
--- a/README.md
+++ b/README.md
@@ -36,16 +36,30 @@ could support non-MRI rubies) and we also needed some bug fixes in
 yajl2, but the maintainer wasn't able to devote enough time to the
 project to make these updates in a timeframe that worked for us.
 
+## Yajl Library Packaging
+
+This library prefers to use the embedded yajl 2.x C library packaged in the
+libyajl2 gem.  In order to use the operating system yajl library (which must be
+yajl 2.x) the environment variable `USE_SYSTEM_LIBYAJL2` can be set before
+installing or bundling libyajl2.  This will force the libyajl2 gem to skip
+compiling its embedded library and the ffi-yajl gem will fallback to using the
+system yajl library.
+
 ## Thanks
 
-This was initially going to be a clean rewrite of an ffi ruby wrapper around yajl2, but as it progressed more and more code was
-pulled in from brianmario's existing yajl-ruby gem, particularly all the c extension code, lots of specs and the benchmarks.  And the
-process of writing this would have been much more difficult without being able to draw heavily from already solved problems in
+This was initially going to be a clean rewrite of an ffi ruby wrapper around
+yajl2, but as it progressed more and more code was pulled in from brianmario's
+existing yajl-ruby gem, particularly all the c extension code, lots of specs
+and the benchmarks.  And the process of writing this would have been much more
+difficult without being able to draw heavily from already solved problems in
 yajl-ruby.
 
 ## License
 
-Given that this draws heavily from the yajl-ruby sources, and could be considered a derivative work, the MIT License from that
-project has been preserved and this source code has deliberately not been dual licensed under Chef's typical Apache License.
-See the [LICENSE](https://github.com/chef/ffi-yajl/blob/master/LICENSE) file in this project.
+Given that this draws heavily from the yajl-ruby sources, and could be
+considered a derivative work, the MIT License from that project has been
+preserved and this source code has deliberately not been dual licensed under
+Chef's typical Apache License.  See the
+[LICENSE](https://github.com/chef/ffi-yajl/blob/master/LICENSE) file in this
+project.
 
diff --git a/Rakefile b/Rakefile
index d8926dc..ae19344 100644
--- a/Rakefile
+++ b/Rakefile
@@ -51,6 +51,13 @@ Rake::ExtensionTask.new do |ext|
   ext.gem_spec = spec
 end
 
+Rake::ExtensionTask.new do |ext|
+  ext.name = 'dlopen'
+  ext.lib_dir = 'lib/ffi_yajl/ext'
+  ext.ext_dir = 'ext/ffi_yajl/ext/dlopen'
+  ext.gem_spec = spec
+end
+
 #
 # test tasks
 #
diff --git a/ext/ffi_yajl/ext/dlopen/dlopen.c b/ext/ffi_yajl/ext/dlopen/dlopen.c
new file mode 100644
index 0000000..eb81a21
--- /dev/null
+++ b/ext/ffi_yajl/ext/dlopen/dlopen.c
@@ -0,0 +1,40 @@
+#include <ruby.h>
+
+#if defined(HAVE_DLFCN_H)
+# include <dlfcn.h>
+#ifndef RTLD_LAZY
+#define RTLD_LAZY 0
+#endif
+#ifndef RTLD_GLOBAL
+#define RTLD_GLOBAL 0
+#endif
+#ifndef RTLD_NOW
+#define RTLD_NOW 0
+#endif
+#else
+# if defined(_WIN32)
+#   include <windows.h>
+#   define dlopen(name,flag) ((void*)LoadLibrary(name))
+#   define dlerror() strerror(rb_w32_map_errno(GetLastError()))
+#   define dlsym(handle,name) ((void*)GetProcAddress((handle),(name)))
+#   define RTLD_LAZY -1
+#   define RTLD_NOW  -1
+#   define RTLD_GLOBAL -1
+# endif
+#endif
+
+static VALUE mFFI_Yajl, mDlopen, mExt;
+
+static VALUE mDlopen_dlopen(VALUE self, VALUE file) {
+  if (dlopen(RSTRING_PTR(file), RTLD_NOW|RTLD_GLOBAL) == NULL) {
+    rb_raise(rb_eArgError, "%s", dlerror());
+  }
+  return Qnil;
+}
+
+void Init_dlopen() {
+  mFFI_Yajl = rb_define_module("FFI_Yajl");
+  mExt = rb_define_module_under(mFFI_Yajl, "Ext");
+  mDlopen = rb_define_module_under(mExt, "Dlopen");
+  rb_define_method(mDlopen, "dlopen", mDlopen_dlopen, 1);
+}
diff --git a/ext/ffi_yajl/ext/dlopen/extconf.rb b/ext/ffi_yajl/ext/dlopen/extconf.rb
new file mode 100644
index 0000000..7e18469
--- /dev/null
+++ b/ext/ffi_yajl/ext/dlopen/extconf.rb
@@ -0,0 +1,15 @@
+require 'mkmf'
+require 'rubygems'
+
+RbConfig::MAKEFILE_CONFIG['CC'] = ENV['CC'] if ENV['CC']
+
+puts $CFLAGS
+puts $LDFLAGS
+
+have_header("dlfcn.h")
+
+have_library("dl", "dlopen")
+
+dir_config 'dlopen'
+
+create_makefile 'ffi_yajl/ext/dlopen'
diff --git a/ffi-yajl.gemspec b/ffi-yajl.gemspec
index 8e6943d..2801527 100644
--- a/ffi-yajl.gemspec
+++ b/ffi-yajl.gemspec
@@ -1,7 +1,6 @@
 gemspec = eval(IO.read(File.expand_path(File.join(File.dirname(__FILE__), "ffi-yajl.gemspec.shared"))))
 
 gemspec.platform = Gem::Platform::RUBY
-gemspec.extensions = %w{ ext/ffi_yajl/ext/encoder/extconf.rb ext/ffi_yajl/ext/parser/extconf.rb }
+gemspec.extensions = %w{ ext/ffi_yajl/ext/encoder/extconf.rb ext/ffi_yajl/ext/parser/extconf.rb ext/ffi_yajl/ext/dlopen/extconf.rb }
 
 gemspec
-
diff --git a/lib/ffi_yajl.rb b/lib/ffi_yajl.rb
index 7f39189..5edc4f6 100644
--- a/lib/ffi_yajl.rb
+++ b/lib/ffi_yajl.rb
@@ -19,10 +19,6 @@ elsif ENV['FORCE_FFI_YAJL'] == "ffi"
   require 'ffi_yajl/ffi'
 elsif RUBY_PLATFORM == "java"
   require 'ffi_yajl/ffi'
-elsif defined?(Yajl)
-  warn "the ffi-yajl and yajl-ruby gems have incompatible C libyajl libs and should not be loaded in the same Ruby VM"
-  warn "falling back to ffi which might work (or might not, no promises)"
-  require 'ffi_yajl/ffi'
 else
   begin
     require 'ffi_yajl/ext'
diff --git a/lib/ffi_yajl/ext.rb b/lib/ffi_yajl/ext.rb
index bc4d410..7ecf172 100644
--- a/lib/ffi_yajl/ext.rb
+++ b/lib/ffi_yajl/ext.rb
@@ -2,62 +2,14 @@ require 'rubygems'
 
 require 'ffi_yajl/encoder'
 require 'ffi_yajl/parser'
-require 'libyajl2'
+require 'ffi_yajl/ext/dlopen'
 require 'ffi_yajl/map_library_name'
 
 module FFI_Yajl
   extend FFI_Yajl::MapLibraryName
+  extend FFI_Yajl::Ext::Dlopen
 
-  libname = map_library_name
-  libpath = File.expand_path(File.join(Libyajl2.opt_path, libname))
-
-  #
-  # FFS, what exactly was so wrong with DL.dlopen that ruby had to get rid of it???
-  #
-
-  def self.try_fiddle_dlopen(libpath)
-    require 'fiddle'
-    if defined?(Fiddle) && Fiddle.respond_to?(:dlopen)
-      ::Fiddle.dlopen(libpath)
-      true
-    else
-      false
-    end
-  rescue LoadError
-    return false
-  end
-
-  def self.try_dl_dlopen(libpath)
-    require 'dl'
-    if defined?(DL) && DL.respond_to?(:dlopen)
-      ::DL.dlopen(libpath)
-      true
-    else
-      false
-    end
-  rescue LoadError
-    return false
-  end
-
-  def self.try_ffi_dlopen(libpath)
-    require 'ffi'
-    require 'rbconfig'
-    extend ::FFI::Library
-    ffi_lib 'dl'
-    attach_function 'dlopen', :dlopen, [:string, :int], :void
-    if  RbConfig::CONFIG['host_os'] =~ /linux/i
-      dlopen libpath, 0x102  # linux: RTLD_GLOBAL | RTLD_NOW
-    else
-      dlopen libpath, 0
-    end
-    true
-  rescue LoadError
-    return false
-  end
-
-  unless try_fiddle_dlopen(libpath) || try_dl_dlopen(libpath) || try_ffi_dlopen(libpath)
-    raise "cannot find dlopen via Fiddle, DL or FFI, what am I supposed to do?"
-  end
+  dlopen_yajl_library
 
   class Parser
     require 'ffi_yajl/ext/parser'
diff --git a/lib/ffi_yajl/ffi.rb b/lib/ffi_yajl/ffi.rb
index 32335ac..b38d5fb 100644
--- a/lib/ffi_yajl/ffi.rb
+++ b/lib/ffi_yajl/ffi.rb
@@ -16,15 +16,7 @@ module FFI_Yajl
 
   extend FFI_Yajl::MapLibraryName
 
-  libname = map_library_name
-  libpath = File.expand_path(File.join(Libyajl2.opt_path, libname))
-
-  if File.file?(libpath)
-    # use our vendored version of libyajl2 if we find it installed
-    ffi_lib libpath
-  else
-    ffi_lib 'yajl'
-  end
+  ffi_open_yajl_library
 
   class YajlCallbacks < ::FFI::Struct
     layout :yajl_null, :pointer,
diff --git a/lib/ffi_yajl/map_library_name.rb b/lib/ffi_yajl/map_library_name.rb
index 3518d07..8723424 100644
--- a/lib/ffi_yajl/map_library_name.rb
+++ b/lib/ffi_yajl/map_library_name.rb
@@ -1,37 +1,110 @@
+# Copyright (c) 2015 Lamont Granquist
+# Copyright (c) 2015 Chef Software, Inc.
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
-require 'ffi_yajl/platform'
+require 'libyajl2'
+
+# Mixin for use in finding the right yajl library on the system.  The 'caller'
+# needs to also mixin either the FFI module or the DLopen module.  Those are
+# deliberately not mixed in to avoid loading the dlopen module in the ffi
+# codepath (which fails on jruby which does not have that C extension).
 
 module FFI_Yajl
   module MapLibraryName
-    include FFI_Yajl::Platform
-    def map_library_name
-      # this is the right answer for the internally built libyajl on windows
-      return "libyajl.so" if windows?
-
-      # this is largely copied from the FFI.map_library_name algorithm, we most likely need
-      # the windows code eventually to support not using the embedded libyajl2-gem
-      libprefix =
-        case RbConfig::CONFIG['host_os'].downcase
-        when /mingw|mswin/
-          ''
-        when /cygwin/
-          'cyg'
-        else
-          'lib'
+
+    private
+
+    # Stub for tests to override the host_os
+    #
+    # @api private
+    # @return Array<String> lower case ruby host_os string
+    def host_os
+      RbConfig::CONFIG['host_os'].downcase
+    end
+
+    # Array of yajl library names on the platform.  Some platforms like Windows
+    # and Mac may have different names/extensions.
+    #
+    # @api private
+    # @return Array<String> Array of yajl library names for platform
+    def library_names
+      case host_os
+      when /mingw|mswin/
+        [ "libyajl.so", "yajl.dll" ]
+      when /cygwin/
+        [ "libyajl.so", "cygyajl.dll" ]
+      when /darwin/
+        [ "libyajl.bundle", "libyajl.dylib" ]
+      else
+        [ "libyajl.so" ]
+      end
+    end
+
+    # Array of yajl library names prepended with the libyajl2 path to use to
+    # load those directly and bypass the system libyajl by default.  Since
+    # these are full paths, this API checks to ensure that the file exists on
+    # the filesystem.  May return an empty array.
+    #
+    # @api private
+    # @return Array<String> Array of full paths to libyajl2 gem libraries
+    def expanded_library_names
+      library_names.map do |libname|
+        pathname = File.expand_path(File.join(Libyajl2.opt_path, libname))
+        pathname if File.file?(pathname)
+      end.compact
+    end
+
+    # Iterate across the expanded library names in the libyajl2-gem and then
+    # attempt to load the system libraries.  Uses the native dlopen extension
+    # that ships in this gem.
+    #
+    # @api private
+    def dlopen_yajl_library
+      found = false
+      ( expanded_library_names + library_names ).each do |libname|
+        begin
+          dlopen(libname)
+          found = true
+          break
+        rescue ArgumentError
         end
-      libsuffix =
-        case RbConfig::CONFIG['host_os'].downcase
-        when /darwin/
-          'bundle'
-        when /linux|bsd|solaris|sunos/
-          'so'
-        when /mingw|mswin|cygwin/
-          'dll'
-        else
-          # Punt and just assume a sane unix (i.e. anything but AIX)
-          'so'
+      end
+      raise "cannot find yajl library for platform" unless found
+    end
+
+    # Iterate across the expanded library names in the libyajl2-gem and attempt
+    # to load them.  If they are missing just use `ffi_lib 'yajl'` to accept
+    # the FFI default algorithm to find the library.
+    #
+    # @api private
+    def ffi_open_yajl_library
+      found = false
+      expanded_library_names.each do |libname|
+        begin
+          ffi_lib libname
+          found = true
+        rescue LoadError
         end
-      libprefix + "yajl" + ".#{libsuffix}"
+      end
+      ffi_lib 'yajl' unless found
     end
   end
 end
diff --git a/spec/ffi_yajl/map_library_name_spec.rb b/spec/ffi_yajl/map_library_name_spec.rb
new file mode 100644
index 0000000..ed4f811
--- /dev/null
+++ b/spec/ffi_yajl/map_library_name_spec.rb
@@ -0,0 +1,96 @@
+
+require 'spec_helper'
+
+class Test
+  extend FFI_Yajl::MapLibraryName
+end
+
+host_os_library_name_mapping = {
+  "mingw"    => [ "libyajl.so", "yajl.dll" ],
+  "mswin"    => [ "libyajl.so", "yajl.dll" ],
+  "cygwin"   => [ "libyajl.so", "cygyajl.dll" ],
+  "darwin"   => [ "libyajl.bundle", "libyajl.dylib" ],
+  "solaris2" => [ "libyajl.so" ],
+  "linux"    => [ "libyajl.so" ],
+  "aix"      => [ "libyajl.so" ],
+  "hpux"     => [ "libyajl.so" ],
+  "netbsd"   => [ "libyajl.so" ],
+  "openbsd"  => [ "libyajl.so" ],
+  "freebsd"  => [ "libyajl.so" ],
+}
+
+describe "FFI_Yajl::MapLibraryName" do
+  let(:libyajl2_opt_path) { "/libyajl2/lib" }
+  before do
+    allow(Libyajl2).to receive(:opt_path).and_return(libyajl2_opt_path)
+  end
+
+  host_os_library_name_mapping.each do |host_os, library_names|
+
+    context "#library_names" do
+      it "maps #{host_os} correctly" do
+        allow(Test).to receive(:host_os).and_return(host_os)
+        expect(Test.send(:library_names)).to eq(library_names)
+      end
+    end
+
+    context "#expanded_library_names" do
+      it "maps #{host_os} correctly" do
+        allow(Test).to receive(:host_os).and_return(host_os)
+        expanded_library_names = []
+        library_names.each do |library_name|
+          path = File.expand_path(File.join(libyajl2_opt_path, library_name))
+          expanded_library_names.push(path)
+          expect(File).to receive(:file?).with(path).and_return(true)
+        end
+        expect(Test.send(:expanded_library_names)).to eq(expanded_library_names)
+      end
+    end
+
+    context "#dlopen_yajl_library" do
+      it "should call dlopen against an expanded library name if it finds it on #{host_os}" do
+        allow(Test).to receive(:host_os).and_return(host_os)
+        library_names.each do |library_name|
+          path = File.expand_path(File.join(libyajl2_opt_path, library_name))
+          allow(File).to receive(:file?).with(path).and_return(true)
+          allow(Test).to receive(:dlopen).with(path).and_return(nil)
+        end
+        Test.send(:dlopen_yajl_library)
+      end
+      it "if dlopen calls all raise it should still use the short names on #{host_os}" do
+        allow(Test).to receive(:host_os).and_return(host_os)
+        library_names.each do |library_name|
+          path = File.expand_path(File.join(libyajl2_opt_path, library_name))
+          allow(File).to receive(:file?).with(path).and_return(true)
+          allow(Test).to receive(:dlopen).with(path).and_raise(ArgumentError)
+        end
+        allow(Test).to receive(:dlopen).with(library_names.first).and_return(nil)
+        Test.send(:dlopen_yajl_library)
+      end
+    end
+
+    context "ffi_open_yajl_library" do
+      it "should call ffi_lib against an expanded library name if it finds it on #{host_os}" do
+        allow(Test).to receive(:host_os).and_return(host_os)
+        library_names.each do |library_name|
+          path = File.expand_path(File.join(libyajl2_opt_path, library_name))
+          allow(File).to receive(:file?).with(path).and_return(true)
+          allow(Test).to receive(:ffi_lib).with(path).and_return(nil)
+        end
+        Test.send(:ffi_open_yajl_library)
+      end
+
+      it "if dlopen calls all raise it should still use 'yajl' on #{host_os}" do
+        allow(Test).to receive(:host_os).and_return(host_os)
+        library_names.each do |library_name|
+          path = File.expand_path(File.join(libyajl2_opt_path, library_name))
+          allow(File).to receive(:file?).with(path).and_return(true)
+          allow(Test).to receive(:ffi_lib).with(path).and_raise(LoadError)
+        end
+        allow(Test).to receive(:ffi_lib).with('yajl').and_return(nil)
+        Test.send(:ffi_open_yajl_library)
+      end
+    end
+
+  end
+end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 71a397b..cb00af6 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -1,5 +1,14 @@
 $: << File.expand_path(File.join(File.dirname( __FILE__ ), "../lib"))
 
+# load yajl-ruby into the same process (tests that both c-libs can be
+# linked in the same process).  this should work, see:
+# http://stackoverflow.com/questions/3232822/linking-with-multiple-versions-of-a-library
+begin
+  require 'yajl'
+rescue LoadError
+  # yajl can't be installed on jruby
+end
+
 require 'ffi_yajl'
 
 RSpec.configure do |c|