New issue
Advanced search Search tips
Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
OS: Windows
Pri: 2
Type: Bug

Blocking:
issue chromium:579196


Previous locations:
chromium:554408


Sign in to add a comment

Consider revising BoringSSL Windows threading code post XP

Project Member Reported by davidben@chromium.org, Nov 11 2015

Issue description

Per http://chrome.blogspot.com/2015/11/updates-to-chrome-platform-support.html, XP support will be gone sometime next year. After that, we can use newer threading APIs. Notably, we can use [0] for CRYPTO_once rather than reimplementing it and [1] for CRYPTO_MUTEX rather than a CRITICAL_SECTION so the read/write locks are actually read/write.

(Unlike pthreads, SRW locks have two unload functions, so we'll need to split CRYPTO_MUTEX_lock back into two functions for read and write, but this shouldn't be much of a nuisance.)

[0] https://msdn.microsoft.com/en-us/library/windows/desktop/aa363808(v=vs.85).aspx
[1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa904937(v=vs.85).aspx
 
Project Member

Comment 1 by davidben@chromium.org, Jan 19 2016

Blocking: chromium:579196
Project Member

Comment 2 by davidben@chromium.org, Apr 11 2016

(Bulk edit to move to BoringSSL project.)
Project Member

Comment 3 by davidben@chromium.org, Apr 11 2016

Project: boringssl
Moved issue chromium:554408 to now be  issue boringssl:37 .
Project Member

Comment 4 by davidben@chromium.org, Apr 11 2016

Components:
Status: Started (was: Assigned)
https://boringssl-review.googlesource.com/#/c/7613/
The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl.git/+/9dadc3b6e1c2d5e2e8a3b1188c905d5541a75df7

commit 9dadc3b6e1c2d5e2e8a3b1188c905d5541a75df7
Author: David Benjamin <davidben@google.com>
Date: Wed Mar 30 23:04:28 2016

Replace CRYPTO_once_t on Windows with INIT_ONCE.

Now that we no longer support Windows XP, this function is available. In doing
so, remove the odd run_once_arg_t union and pass in a pointer to a function
pointer which is cleaner and still avoids C's silly rule where function
pointers can't be placed in a void*.

BUG= 37 

Change-Id: I44888bb3779dacdb660706debd33888ca389ebd5
Reviewed-on: https://boringssl-review.googlesource.com/7613
Reviewed-by: David Benjamin <davidben@google.com>

[modify] https://crrev.com/9dadc3b6e1c2d5e2e8a3b1188c905d5541a75df7/crypto/internal.h
[modify] https://crrev.com/9dadc3b6e1c2d5e2e8a3b1188c905d5541a75df7/crypto/thread_win.c

Project Member

Comment 6 by davidben@chromium.org, Apr 19 2016

Chromium's now changed their CRITICAL_SECTIONs to SRWLOCKs so this is apparently a desirable thing even independent of read/write vs mutex. (Though we may as well make it read/write since it's free.)
The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl.git/+/29270dea85741f69bd080bea6b28a83476c2bc91

commit 29270dea85741f69bd080bea6b28a83476c2bc91
Author: David Benjamin <davidben@google.com>
Date: Tue May 24 15:28:36 2016

Split unlock functions into read/write variants.

Windows SRWLOCK requires you call different functions here. Split
them up in preparation for switching Windows from CRITICAL_SECTION.

BUG= 37 

Change-Id: I7b5c6a98eab9ae5bb0734b805cfa1ff334918f35
Reviewed-on: https://boringssl-review.googlesource.com/8080
Reviewed-by: Adam Langley <agl@google.com>

[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/bn/montgomery.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/err/err.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/ex_data.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/internal.h
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/obj/obj.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/rand/urandom.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/refcount_lock.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/rsa/rsa_impl.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/thread_none.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/thread_pthread.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/thread_win.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/x509/by_dir.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/x509/x509_lu.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/x509/x_crl.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/x509/x_pubkey.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/x509v3/pcy_cache.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/crypto/x509v3/v3_purp.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/ssl/ssl_lib.c
[modify] https://crrev.com/29270dea85741f69bd080bea6b28a83476c2bc91/ssl/ssl_session.c

The following revision refers to this bug:
  https://boringssl.googlesource.com/boringssl.git/+/156edfe5361ad635c091265eb8035f2c310371d3

commit 156edfe5361ad635c091265eb8035f2c310371d3
Author: David Benjamin <davidben@google.com>
Date: Tue May 24 15:41:11 2016

Switch Windows CRYPTO_MUTEX implementation to SRWLOCK.

Now that we no longer support Windows XP, this is available.
Unfortunately, the public header version of CRYPTO_MUTEX means we
still can't easily merge CRYPTO_MUTEX and CRYPTO_STATIC_MUTEX.

BUG= 37 

Change-Id: If309de3f06e0854c505083b72fd64d1dbb3f4563
Reviewed-on: https://boringssl-review.googlesource.com/8081
Reviewed-by: Adam Langley <agl@google.com>

[modify] https://crrev.com/156edfe5361ad635c091265eb8035f2c310371d3/crypto/internal.h
[modify] https://crrev.com/156edfe5361ad635c091265eb8035f2c310371d3/crypto/thread_win.c
[modify] https://crrev.com/156edfe5361ad635c091265eb8035f2c310371d3/include/openssl/thread.h

Project Member

Comment 9 by davidben@chromium.org, May 31 2016

Status: Fixed (was: Started)

Sign in to add a comment