Always initialize session cache locks lazily.

Always initialize session cache locks lazily.

Code analysis shows it is sufficient to initialize session cache
locks lazily before we create an sslSocket because the two locks
are always used with an sslSocket 'ss', except when we clear
session caches.  This matches our expectation that only SSL sockets
use the SSL session cache.

R=rsleevi@chromium.org
BUG=131622
TEST=no crashes due to null pointer dereference inside NSS SSL code.

[wtc, 2012-06-13 23:08:21]

rsleevi: I decided to create an NSS patch that can be upstreamed.
The net::EnableSSLServerSockets() calls added in
https://chromiumcodereview.appspot.com/10543106/ leaked
properties of NSS that should be hidden.

Although this patch changes NSS to always initialize the
locks lazily, it reduces the number of PR_CallOnce calls
significantly, to whenever we create an sslSocket.  The
original code calls PR_CallOnce whenever it needs to use
those locks.

I will open an upstream NSS bug.

https://chromiumcodereview.appspot.com/10539144/diff/1/net/third_party/nss/ss...
File net/third_party/nss/ssl/ssl3con.c (left):

https://chromiumcodereview.appspot.com/10539144/diff/1/net/third_party/nss/ss...
net/third_party/nss/ssl/ssl3con.c:4542: ssl_FreeSessionCacheLocks();

The ssl_FreeSessionCacheLocks function is used when we
initialize the locks early.  So ssl_FreeSessionCacheLocks
is also deleted.

https://chromiumcodereview.appspot.com/10539144/diff/1/net/third_party/nss/ss...
File net/third_party/nss/ssl/ssl3con.c (right):

https://chromiumcodereview.appspot.com/10539144/diff/1/net/third_party/nss/ss...
net/third_party/nss/ssl/ssl3con.c:4527: return SECSuccess;	/* lock was never
initialized */

Note that SSL3_ShutdownServerCache does a null check on
symWrapKeysLock rather than initializing the locks.  I
copied this solution to SSL_ClearSessionCache.

[Ryan Sleevi, 2012-06-14 01:23:34]

This won't help use_system_ssl users (should we just remove this support
entirely for Linux? Shall we talk with phajdan.jr?)

At a high level view, I agree that there's some potential abstraction leaking -
but do you think it's any different then the fact that code like content/ or the
renderer/ have to ensure NSS is initialized in the sandbox?

We can certainly hide the dependency through wrapper functions
(remoting::InitRemoting() -> net::EnableSSLServerSockets, etc), but I don't
think the idea of "functions you need to call early" is particularly anathema
(eg: CommandLine, AtExitManager, etc)

Is PR_CallOnce somehow heavier than our LazyInstance and friends?

If so, we should consider upstreaming that. LazyInstance uses an atomic compare
and swap on the platforms that support it (eg: all the ones we care about). Even
if it means PR_CallOnce needs to be slow on platforms like CE?Solaris/insert
edge case here, I think we'd be better off optimizing PR_CallOnce and calling it
when needed rather than relying on the correct order of operations.

https://chromiumcodereview.appspot.com/10539144/diff/1006/net/third_party/nss...
File net/third_party/nss/ssl/sslsnce.c (left):

https://chromiumcodereview.appspot.com/10539144/diff/1006/net/third_party/nss...
net/third_party/nss/ssl/sslsnce.c:1494: ssl_InitSessionCacheLocks(PR_FALSE);
I don't believe this is correct.

Because the server process needs to configure the cache, it should be
initializing the cache locks, not lazily in the socket import.

https://chromiumcodereview.appspot.com/10539144/diff/1006/net/third_party/nss...
File net/third_party/nss/ssl/sslsock.c (right):

https://chromiumcodereview.appspot.com/10539144/diff/1006/net/third_party/nss...
net/third_party/nss/ssl/sslsock.c:1406: */
See my other comment - I don't think this is correct/expected.