-
-
Notifications
You must be signed in to change notification settings - Fork 925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Backport securerandom #4149
Backport securerandom #4149
Conversation
…ocation shaves off 10-15% of Thread.new/start : ``` Rehearsal ---------------------------------------------------------- Thread.new [100000x] 12.800000 6.850000 19.650000 ( 10.392600) Thread.start [100000x] 12.670000 5.680000 18.350000 ( 9.625846) ------------------------------------------------ total: 38.000000sec user system total real Thread.new [100000x] 12.780000 5.760000 18.540000 ( 9.620000) Thread.start [100000x] 12.490000 5.810000 18.300000 ( 9.524975) ``` ``` Rehearsal ---------------------------------------------------------- Thread.new [100000x] 11.040000 5.560000 16.600000 ( 8.708792) Thread.start [100000x] 10.740000 5.520000 16.260000 ( 8.400990) ------------------------------------------------ total: 32.860000sec user system total real Thread.new [100000x] 10.840000 5.620000 16.460000 ( 8.506339) Thread.start [100000x] 10.460000 5.560000 16.020000 ( 8.323819) ```
I botched the previous patch a bit by unconditionally re-assigning the secureRandom local to a default JDK new SecureRandom. This could cause systems without the default preferred PRNG (NativePRNGNonBlocking, Java 8+) to have slower thread startup and/or random number generation.
4415110
to
2af75fa
Compare
isn't 8fb4b93 "dangerous" to back-port (changing a previously initialized field on |
@@ -146,6 +146,7 @@ | |||
public static final Option<Boolean> REIFY_VARIABLES = bool(MISCELLANEOUS, "reify.variables", false, "Attempt to expand instance vars into Java fields"); | |||
public static final Option<Boolean> PREFER_IPV4 = bool(MISCELLANEOUS, "net.preferIPv4", true, "Prefer IPv4 network stack"); | |||
public static final Option<Boolean> FCNTL_LOCKING = bool(MISCELLANEOUS, "file.flock.fcntl", true, "Use fcntl rather than flock for File#flock"); | |||
public static final Option<String> PREFERRED_PRNG = string(MISCELLANEOUS, "preferred.prng", "NativePRNGNonBlocking", "Maintain children static scopes to support scope dumping."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the help message for the option seems like a copy 🍝 left over
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed...I'll fix that on both master and in this PR.
A little dangerous, I agree. However the internals of ThreadContext have never been a blessed API. I know there might be people poking at them, but we've never shown people examples that use TC internals and we have generally discouraged messing with it. I'm not too concerned, especially since it helps thread startup times (important for Fiber and Enumerator#next as well). @enebo? |
@takanuva This is the backported SecureRandom changes I mentioned. Since you have an environment with entropy problems, perhaps you could test out a HEAD build of JRuby 1.7 for us once we merge? It would be REALLY nice to know that this actually fixes your issue without installing an entropy-generator. Up to now we've just assumed it does, since it's hard to simulate an environment with entropy-starvation problems. |
@bbrowning or @mkristian can you see a problem with this change to threadcontext not having securerandom initialized? I personally think the potential for this breaking stuff could only happen in a deep extension but not very likely. |
@enebo I don't know of anything that uses securerandom directly like this. I agree it would have to be an extension really digging deep to get hit by this. While it is possible that one exists somewhere, a quick GitHub search didn't turn up anything obvious. |
... no (more) worries from me than - thanks everyone for looking into it! |
Sure, @headius, just give me a few days and I'll gladly test that for you guys. 😄 |
This PR backports recent SecureRandom entropy fixes from 9k to 1.7. See #1405.
We had a 1.7 user in #jruby today with entropy problems.