I just hit this (got spooky leftover thread warning running test on OS
X). I think we should fix it.
I like the initial approach: let's not use MessageDigest at all
(import our own MD5, that does not spawn threads!!). Sure it's code
dup but it's tiny and it mitigates risk so I think it's well worth
In general Lucene should not use "interesting" (risky) parts of the
JVM/Java if we can avoid it w/o much cost, and this is a really silly
reason to be using MessageDigest (similar to our now-gone crazy usage
of ManagementFactory just to acquire a test lock). We are a search
library! We must use the bare minimum of the OS/filesystem/JVM that
In fact in this case... can't we nuke DIGESTER altogether? Lucene now
stores lock files in the index dir by default as "write.lock". We
only need this digest if you change that dir. So, if your app somehow
wants to put the lock file elsewhere (unusual), it should be up to you
to name it "uniquely" relative to other IWs storing locks in the same
dir (we can do this under a separate issue).
And not using SecureRandom to create temp files is a no-brainer – the
builtin File.createTempFile must be secure, by design, but we
obviously don't need that here. I've had awful problems in the past w/
SecureRandom (because my machine didn't have enough true
randomness!). Again: Lucene should only use what's we really need.
I think we can remove the controversial "interrupt the weird OS X
PKCS11 thread" from the patch since serialization is now gone? I'd
like to know if this thread suddenly pops up again in our tests... and
I agree it's dangerous to interrupt this thread (it could then cause
weird failures in subsequent tests, eg if the thread doesn't restart).
One thing: I don't like the empty catch blocks /* cannot happen */. Even if this is the case, please throw at least a RuntimException
+1 – I like this idea (I don't do it now but I'll try to going
forward). Defensive programming...