Commit b4024d4b1505f3c87b4c0b1ed59b8850aeea719f
Ruby 1.9: do not unlock the GIL unless the bcrypt cost is sufficiently high.
Locking/unlocking the GIL incurs some overhead as well so we want to avoid that for cheap operations. (merged with JRuby changes) Signed-off-by: Coda Hale <coda.hale@gmail.com>Hongli Lai (Phusion) authored on 8/5/2009, 7:32:55 AM
Coda Hale committed on 8/12/2009, 10:09:46 PM
Parent: 8caa1cce2f45c862ce127c2cbdeea0490b55d57f
Files changed
ext/bcrypt_ext.c | changed |
lib/bcrypt.rb | changed |
spec/bcrypt/engine_spec.rb | changed |
ext/bcrypt_ext.c | ||
---|---|---|
@@ -16,10 +16,12 @@ | ||
16 | 16 | |
17 | 17 | |
18 | 18 | |
19 | 19 | /* When on Ruby 1.9+, we will want to unlock the GIL while performing |
20 | - * expensive calculations, for greater concurrency. | |
20 | + * expensive calculations, for greater concurrency. Do not do this for | |
21 | + * cheap calculations because locking/unlocking the GIL incurs some overhead as well. | |
21 | 22 | */ |
23 | + | |
22 | 24 | |
23 | 25 | typedef struct { |
24 | 26 | char *output; |
25 | 27 | const char *key; |
@@ -44,38 +46,42 @@ | ||
44 | 46 | } |
45 | 47 | |
46 | 48 | /* Given a secret and a salt, generates a salted hash (which you can then store safely). |
47 | 49 | */ |
48 | -static VALUE bc_crypt(VALUE self, VALUE key, VALUE salt) { | |
50 | +static VALUE bc_crypt(VALUE self, VALUE key, VALUE salt, VALUE cost) { | |
49 | 51 | const char * safeguarded = RSTRING_PTR(key) ? RSTRING_PTR(key) : ""; |
50 | 52 | char output[BCRYPT_OUTPUT_SIZE]; |
51 | 53 | |
52 | 54 | |
53 | - BCryptArguments args; | |
54 | - VALUE ret; | |
55 | + int icost = NUM2INT(cost); | |
56 | + if (icost >= GIL_UNLOCK_COST_THRESHOLD) { | |
57 | + BCryptArguments args; | |
58 | + VALUE ret; | |
55 | 59 | |
56 | - args.output = output; | |
57 | - args.key = safeguarded; | |
58 | - args.salt = RSTRING_PTR(salt); | |
59 | - ret = rb_thread_blocking_region(bcrypt_wrapper, &args, RUBY_UBF_IO, 0); | |
60 | - if (ret != (VALUE) 0) { | |
61 | - return rb_str_new2(output); | |
62 | - } else { | |
63 | - return Qnil; | |
60 | + args.output = output; | |
61 | + args.key = safeguarded; | |
62 | + args.salt = RSTRING_PTR(salt); | |
63 | + ret = rb_thread_blocking_region(bcrypt_wrapper, &args, RUBY_UBF_IO, 0); | |
64 | + if (ret != (VALUE) 0) { | |
65 | + return rb_str_new2(output); | |
66 | + } else { | |
67 | + return Qnil; | |
68 | + } | |
64 | 69 | } |
65 | - | |
66 | - if (bcrypt(output, safeguarded, (char *)RSTRING_PTR(salt)) != NULL) { | |
67 | - return rb_str_new2(output); | |
68 | - } else { | |
69 | - return Qnil; | |
70 | - } | |
70 | + /* otherwise, fallback to the non-GIL-unlocking code, just like on Ruby 1.8 */ | |
71 | 71 | |
72 | + | |
73 | + if (bcrypt(output, safeguarded, (char *)RSTRING_PTR(salt)) != NULL) { | |
74 | + return rb_str_new2(output); | |
75 | + } else { | |
76 | + return Qnil; | |
77 | + } | |
72 | 78 | } |
73 | 79 | |
74 | 80 | /* Create the BCrypt and BCrypt::Engine modules, and populate them with methods. */ |
75 | 81 | void Init_bcrypt_ext(){ |
76 | 82 | mBCrypt = rb_define_module("BCrypt"); |
77 | 83 | cBCryptEngine = rb_define_class_under(mBCrypt, "Engine", rb_cObject); |
78 | 84 | |
79 | 85 | rb_define_singleton_method(cBCryptEngine, "__bc_salt", bc_salt, 2); |
80 | - rb_define_singleton_method(cBCryptEngine, "__bc_crypt", bc_crypt, 2); | |
86 | + rb_define_singleton_method(cBCryptEngine, "__bc_crypt", bc_crypt, 3); | |
81 | 87 | } |
lib/bcrypt.rb | ||
---|---|---|
@@ -37,15 +37,19 @@ | ||
37 | 37 | end |
38 | 38 | |
39 | 39 | # Given a secret and a valid salt (see BCrypt::Engine.generate_salt) calculates |
40 | 40 | # a bcrypt() password hash. |
41 | - def self.hash_secret(secret, salt) | |
41 | + def self.hash_secret(secret, salt, cost = nil) | |
42 | 42 | if valid_secret?(secret) |
43 | 43 | if valid_salt?(salt) |
44 | + if cost.nil? | |
45 | + cost = autodetect_cost(salt) | |
46 | + end | |
47 | + | |
44 | 48 | if RUBY_PLATFORM == "java" |
45 | 49 | Java.bcrypt_jruby.BCrypt.hashpw(secret.to_s, salt.to_s) |
46 | 50 | else |
47 | - __bc_crypt(secret.to_s, salt) | |
51 | + __bc_crypt(secret.to_s, salt, cost) | |
48 | 52 | end |
49 | 53 | else |
50 | 54 | raise Errors::InvalidSalt.new("invalid salt") |
51 | 55 | end |
@@ -100,8 +104,13 @@ | ||
100 | 104 | end_time = Time.now - start_time |
101 | 105 | return i if end_time * 1_000 > upper_time_limit_in_ms |
102 | 106 | end |
103 | 107 | end |
108 | + | |
109 | + # Autodetects the cost from the salt string. | |
110 | + def self.autodetect_cost(salt) | |
111 | + salt[4..5].to_i | |
112 | + end | |
104 | 113 | end |
105 | 114 | |
106 | 115 | # A password management class which allows you to safely store users' passwords and compare them. |
107 | 116 | # |
@@ -144,9 +153,9 @@ | ||
144 | 153 | # Example: |
145 | 154 | # |
146 | 155 | # @password = BCrypt::Password.create("my secret", :cost => 13) |
147 | 156 | def create(secret, options = { :cost => BCrypt::Engine::DEFAULT_COST }) |
148 | - Password.new(BCrypt::Engine.hash_secret(secret, BCrypt::Engine.generate_salt(options[:cost]))) | |
157 | + Password.new(BCrypt::Engine.hash_secret(secret, BCrypt::Engine.generate_salt(options[:cost]), options[:cost])) | |
149 | 158 | end |
150 | 159 | end |
151 | 160 | |
152 | 161 | # Initializes a BCrypt::Password instance with the data from a stored hash. |
spec/bcrypt/engine_spec.rb | ||
---|---|---|
@@ -26,8 +26,18 @@ | ||
26 | 26 | lambda { BCrypt::Engine.generate_salt(-1) }.should raise_error(BCrypt::Errors::InvalidCost) |
27 | 27 | end |
28 | 28 | end |
29 | 29 | |
30 | +context "Autodetecting of salt cost" do | |
31 | + | |
32 | + specify "should work" do | |
33 | + BCrypt::Engine.autodetect_cost("$2a$08$hRx2IVeHNsTSYYtUWn61Ou").should == 8 | |
34 | + BCrypt::Engine.autodetect_cost("$2a$05$XKd1bMnLgUnc87qvbAaCUu").should == 5 | |
35 | + BCrypt::Engine.autodetect_cost("$2a$13$Lni.CZ6z5A7344POTFBBV.").should == 13 | |
36 | + end | |
37 | + | |
38 | +end | |
39 | + | |
30 | 40 | context "Generating BCrypt hashes" do |
31 | 41 | |
32 | 42 | class MyInvalidSecret |
33 | 43 | undef to_s |
Built with git-ssb-web