From ba4f3612b6956473f700c068fb1a146f3fe1c9f3 Mon Sep 17 00:00:00 2001 From: Brian Lopez Date: Sun, 17 Oct 2010 16:34:09 -0700 Subject: [PATCH 1/7] move to rspec2 --- .rspec | 2 ++ spec/spec.opts | 2 -- spec/spec_helper.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) create mode 100644 .rspec delete mode 100644 spec/spec.opts diff --git a/.rspec b/.rspec new file mode 100644 index 0000000..83e2753 --- /dev/null +++ b/.rspec @@ -0,0 +1,2 @@ +--format documentation +--colour diff --git a/spec/spec.opts b/spec/spec.opts deleted file mode 100644 index e0f74bb..0000000 --- a/spec/spec.opts +++ /dev/null @@ -1,2 +0,0 @@ ---format specdoc ---colour diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 39da1e4..cdc842f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,7 +4,7 @@ require 'rubygems' require 'mysql2' require 'timeout' -Spec::Runner.configure do |config| +RSpec.configure do |config| config.before(:all) do client = Mysql2::Client.new :database => 'test' client.query %[ From 0a7e7ee4755e6da05b9e3ac525d4355ae6be6c7b Mon Sep 17 00:00:00 2001 From: Brian Lopez Date: Sun, 17 Oct 2010 16:34:49 -0700 Subject: [PATCH 2/7] centralize closing and freeing logic for connections --- ext/mysql2/client.c | 76 ++++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 39 deletions(-) diff --git a/ext/mysql2/client.c b/ext/mysql2/client.c index ef4ecc8..f95bd53 100644 --- a/ext/mysql2/client.c +++ b/ext/mysql2/client.c @@ -104,54 +104,52 @@ static VALUE nogvl_connect(void *ptr) { return client ? Qtrue : Qfalse; } -static void rb_mysql_client_free(void * ptr) { - mysql_client_wrapper *wrapper = (mysql_client_wrapper *)ptr; - - /* - * we'll send a QUIT message to the server, but that message is more of a - * formality than a hard requirement since the socket is getting shutdown - * anyways, so ensure the socket write does not block our interpreter - */ - int fd = wrapper->client->net.fd; - - if (fd >= 0) { - /* - * if the socket is dead we have no chance of blocking, - * so ignore any potential fcntl errors since they don't matter - */ -#ifndef _WIN32 - int flags = fcntl(fd, F_GETFL); - if (flags > 0 && !(flags & O_NONBLOCK)) - fcntl(fd, F_SETFL, flags | O_NONBLOCK); -#else - u_long iMode = 1; - ioctlsocket(fd, FIONBIO, &iMode); -#endif - } - - /* It's safe to call mysql_close() on an already closed connection. */ - if (!wrapper->closed) { - mysql_close(wrapper->client); - if (!wrapper->freed) { - free(wrapper->client); - } - } - xfree(ptr); -} - -static VALUE nogvl_close(void * ptr) { +static VALUE nogvl_close(void *ptr) { mysql_client_wrapper *wrapper = ptr; if (!wrapper->closed) { wrapper->closed = 1; + + /* + * we'll send a QUIT message to the server, but that message is more of a + * formality than a hard requirement since the socket is getting shutdown + * anyways, so ensure the socket write does not block our interpreter + */ + int fd = wrapper->client->net.fd; + + if (fd >= 0) { + /* + * if the socket is dead we have no chance of blocking, + * so ignore any potential fcntl errors since they don't matter + */ + #ifndef _WIN32 + int flags = fcntl(fd, F_GETFL); + if (flags > 0 && !(flags & O_NONBLOCK)) + fcntl(fd, F_SETFL, flags | O_NONBLOCK); + #else + u_long iMode = 1; + ioctlsocket(fd, FIONBIO, &iMode); + #endif + } + mysql_close(wrapper->client); wrapper->client->net.fd = -1; - if (!wrapper->freed) { - free(wrapper->client); - } + } + + if (!wrapper->freed) { + wrapper->freed = 1; + free(wrapper->client); } return Qnil; } +static void rb_mysql_client_free(void * ptr) { + mysql_client_wrapper *wrapper = (mysql_client_wrapper *)ptr; + + nogvl_close(wrapper); + + xfree(ptr); +} + static VALUE allocate(VALUE klass) { VALUE obj; mysql_client_wrapper * wrapper; From 22c9ff48a8625b90284f7ed72d50d97ed8735e11 Mon Sep 17 00:00:00 2001 From: Brian Lopez Date: Sun, 17 Oct 2010 16:51:14 -0700 Subject: [PATCH 3/7] some more rspec2 changes, organize rake tasks --- Rakefile | 37 ------------------------------------- spec/spec_helper.rb | 3 ++- tasks/jeweler.rake | 17 +++++++++++++++++ tasks/rspec.rake | 16 ++++++++++++++++ 4 files changed, 35 insertions(+), 38 deletions(-) create mode 100644 tasks/jeweler.rake create mode 100644 tasks/rspec.rake diff --git a/Rakefile b/Rakefile index df610fe..7bb616b 100644 --- a/Rakefile +++ b/Rakefile @@ -1,42 +1,5 @@ # encoding: UTF-8 -begin - require 'jeweler' - JEWELER = Jeweler::Tasks.new do |gem| - gem.name = "mysql2" - gem.summary = "A simple, fast Mysql library for Ruby, binding to libmysql" - gem.email = "seniorlopez@gmail.com" - gem.homepage = "http://github.com/brianmario/mysql2" - gem.authors = ["Brian Lopez"] - gem.require_paths = ["lib", "ext"] - gem.extra_rdoc_files = `git ls-files *.rdoc`.split("\n") - gem.files = `git ls-files`.split("\n") - gem.extensions = ["ext/mysql2/extconf.rb"] - gem.files.include %w(lib/jeweler/templates/.document lib/jeweler/templates/.gitignore) - # gem.rubyforge_project = "mysql2" - end -rescue LoadError - puts "Jeweler, or one of its dependencies, is not available. Install it with: sudo gem install jeweler -s http://gems.github.com" -end - require 'rake' -require 'spec/rake/spectask' - -desc "Run all examples with RCov" -Spec::Rake::SpecTask.new('spec:rcov') do |t| - t.spec_files = FileList['spec/'] - t.rcov = true - t.rcov_opts = lambda do - IO.readlines("spec/rcov.opts").map {|l| l.chomp.split " "}.flatten - end -end -Spec::Rake::SpecTask.new('spec') do |t| - t.spec_files = FileList['spec/'] - t.spec_opts << '--options' << 'spec/spec.opts' - t.verbose = true - t.warning = true -end - -task :default => :spec # Load custom tasks Dir['tasks/*.rake'].sort.each { |f| load f } diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cdc842f..a0e5770 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,6 +1,7 @@ # encoding: UTF-8 -require 'rubygems' +$LOAD_PATH.unshift File.expand_path("../../lib", __FILE__) +require 'rspec' require 'mysql2' require 'timeout' diff --git a/tasks/jeweler.rake b/tasks/jeweler.rake new file mode 100644 index 0000000..e04979c --- /dev/null +++ b/tasks/jeweler.rake @@ -0,0 +1,17 @@ +begin + require 'jeweler' + JEWELER = Jeweler::Tasks.new do |gem| + gem.name = "mysql2" + gem.summary = "A simple, fast Mysql library for Ruby, binding to libmysql" + gem.email = "seniorlopez@gmail.com" + gem.homepage = "http://github.com/brianmario/mysql2" + gem.authors = ["Brian Lopez"] + gem.require_paths = ["lib", "ext"] + gem.extra_rdoc_files = `git ls-files *.rdoc`.split("\n") + gem.files = `git ls-files`.split("\n") + gem.extensions = ["ext/mysql2/extconf.rb"] + gem.files.include %w(lib/jeweler/templates/.document lib/jeweler/templates/.gitignore) + end +rescue LoadError + puts "jeweler, or one of its dependencies, is not available. Install it with: sudo gem install jeweler" +end \ No newline at end of file diff --git a/tasks/rspec.rake b/tasks/rspec.rake new file mode 100644 index 0000000..a6628e8 --- /dev/null +++ b/tasks/rspec.rake @@ -0,0 +1,16 @@ +begin + require 'rspec' + require 'rspec/core/rake_task' + + desc "Run all examples with RCov" + RSpec::Core::RakeTask.new('spec:rcov') do |t| + t.rcov = true + end + RSpec::Core::RakeTask.new('spec') do |t| + t.verbose = true + end + + task :default => :spec +rescue LoadError + puts "rspec, or one of its dependencies, is not available. Install it with: sudo gem install rspec" +end \ No newline at end of file From 0ae583fe64cd481d5a54829be3f34a75342d7bcd Mon Sep 17 00:00:00 2001 From: Brian Lopez Date: Sun, 17 Oct 2010 16:59:50 -0700 Subject: [PATCH 4/7] since we free the connection on close, there's no reason to have both flags --- ext/mysql2/client.c | 6 +----- ext/mysql2/client.h | 1 - 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/ext/mysql2/client.c b/ext/mysql2/client.c index f95bd53..379adff 100644 --- a/ext/mysql2/client.c +++ b/ext/mysql2/client.c @@ -133,12 +133,9 @@ static VALUE nogvl_close(void *ptr) { mysql_close(wrapper->client); wrapper->client->net.fd = -1; - } - - if (!wrapper->freed) { - wrapper->freed = 1; free(wrapper->client); } + return Qnil; } @@ -157,7 +154,6 @@ static VALUE allocate(VALUE klass) { wrapper->encoding = Qnil; wrapper->active = 0; wrapper->closed = 0; - wrapper->freed = 0; wrapper->client = (MYSQL*)malloc(sizeof(MYSQL)); return obj; } diff --git a/ext/mysql2/client.h b/ext/mysql2/client.h index fca99c4..d5c2993 100644 --- a/ext/mysql2/client.h +++ b/ext/mysql2/client.h @@ -35,7 +35,6 @@ typedef struct { VALUE encoding; short int active; short int closed; - short int freed; MYSQL *client; } mysql_client_wrapper; From 3b6229771a6d0b8e949e920ac3d8c4d8eb4e433a Mon Sep 17 00:00:00 2001 From: Brian Lopez Date: Sun, 17 Oct 2010 17:34:21 -0700 Subject: [PATCH 5/7] remove benchmark that tested code that has since been removed --- benchmark/thread_alone.rb | 20 -------------------- 1 file changed, 20 deletions(-) delete mode 100644 benchmark/thread_alone.rb diff --git a/benchmark/thread_alone.rb b/benchmark/thread_alone.rb deleted file mode 100644 index c6b4c1e..0000000 --- a/benchmark/thread_alone.rb +++ /dev/null @@ -1,20 +0,0 @@ -# encoding: UTF-8 -$LOAD_PATH.unshift File.expand_path(File.dirname(__FILE__) + '/../lib') - -require 'rubygems' -require 'benchmark' -require 'mysql2' - -iterations = 1000 -client = Mysql2::Client.new(:host => "localhost", :username => "root", :database => "test") -query = lambda{ iterations.times{ client.query("SELECT mysql2_test.* FROM mysql2_test") } } -Benchmark.bmbm do |x| - x.report('select') do - query.call - end - x.report('rb_thread_select') do - thread = Thread.new{ sleep(10) } - query.call - thread.kill - end -end \ No newline at end of file From 410e9144113cfa77153c35c039f169e238edf5f3 Mon Sep 17 00:00:00 2001 From: Brian Lopez Date: Sun, 17 Oct 2010 17:34:34 -0700 Subject: [PATCH 6/7] only run the EM specs if EM is installed --- spec/em/em_spec.rb | 76 ++++++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/spec/em/em_spec.rb b/spec/em/em_spec.rb index 1254c4c..429acf1 100644 --- a/spec/em/em_spec.rb +++ b/spec/em/em_spec.rb @@ -1,45 +1,49 @@ # encoding: UTF-8 -require 'spec_helper' -require 'mysql2/em' +if defined? EventMachine + require 'spec_helper' + require 'mysql2/em' -describe Mysql2::EM::Client do - it "should support async queries" do - results = [] - EM.run do - client1 = Mysql2::EM::Client.new - defer1 = client1.query "SELECT sleep(0.1) as first_query" - defer1.callback do |result| - results << result.first - EM.stop_event_loop - end - - client2 = Mysql2::EM::Client.new - defer2 = client2.query "SELECT sleep(0.025) second_query" - defer2.callback do |result| - results << result.first - end - end - - results[0].keys.should include("second_query") - results[1].keys.should include("first_query") - end - - it "should support queries in callbacks" do - results = [] - EM.run do - client = Mysql2::EM::Client.new - defer1 = client.query "SELECT sleep(0.025) as first_query" - defer1.callback do |result| - results << result.first - defer2 = client.query "SELECT sleep(0.025) as second_query" - defer2.callback do |result| + describe Mysql2::EM::Client do + it "should support async queries" do + results = [] + EM.run do + client1 = Mysql2::EM::Client.new + defer1 = client1.query "SELECT sleep(0.1) as first_query" + defer1.callback do |result| results << result.first EM.stop_event_loop end + + client2 = Mysql2::EM::Client.new + defer2 = client2.query "SELECT sleep(0.025) second_query" + defer2.callback do |result| + results << result.first + end end + + results[0].keys.should include("second_query") + results[1].keys.should include("first_query") end - results[0].keys.should include("first_query") - results[1].keys.should include("second_query") + it "should support queries in callbacks" do + results = [] + EM.run do + client = Mysql2::EM::Client.new + defer1 = client.query "SELECT sleep(0.025) as first_query" + defer1.callback do |result| + results << result.first + defer2 = client.query "SELECT sleep(0.025) as second_query" + defer2.callback do |result| + results << result.first + EM.stop_event_loop + end + end + end + + results[0].keys.should include("first_query") + results[1].keys.should include("second_query") + end end -end +else + puts "EventMachine not installed, skipping the specs that use it" +end \ No newline at end of file From 3c6959a8bbd8846230d3d8a71f09bb2faeaa7268 Mon Sep 17 00:00:00 2001 From: Brian Lopez Date: Sun, 17 Oct 2010 17:35:41 -0700 Subject: [PATCH 7/7] prevent the GC from freeing a connection that hasn't been allocated yet --- ext/mysql2/client.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ext/mysql2/client.c b/ext/mysql2/client.c index 379adff..ab4cb66 100644 --- a/ext/mysql2/client.c +++ b/ext/mysql2/client.c @@ -153,7 +153,7 @@ static VALUE allocate(VALUE klass) { obj = Data_Make_Struct(klass, mysql_client_wrapper, rb_mysql_client_mark, rb_mysql_client_free, wrapper); wrapper->encoding = Qnil; wrapper->active = 0; - wrapper->closed = 0; + wrapper->closed = 1; wrapper->client = (MYSQL*)malloc(sizeof(MYSQL)); return obj; } @@ -517,6 +517,7 @@ static VALUE init_connection(VALUE self) { return rb_raise_mysql2_error(wrapper->client); } + wrapper->closed = 0; return self; }