Yeah, "un-initialization functions" meant to mean "any of initialization or
uninitialization functions". I didn't communicate my point correctly.
> As grpc does not do any shutdown on OpenSSL, I can ensure that my
application is only cleaning it up after I am done with grpc.
... which is going to work swell if someone loads/unloads grpc dynamically
;-)
> As for your latest remark that SSL-library-init functions are not
threadsafe, they could also be wrapped inside the callback check.
Oh! Good thing you reminded me -- "callback check" in this pull request
isn't threadsafe. :-) I.e. this:
if (!CRYPTO_get_locking_callback()) {
int num_locks = CRYPTO_num_locks();
GPR_ASSERT(num_locks > 0);
g_openssl_mutexes = static_cast<gpr_mu*>(
gpr_malloc(static_cast<size_t>(num_locks) * sizeof(gpr_mu)));
for (int i = 0; i < num_locks; i++) {
gpr_mu_init(&g_openssl_mutexes[i]);
}
CRYPTO_set_locking_callback(openssl_locking_cb);
CRYPTO_set_id_callback(openssl_thread_id_cb);
} else {
gpr_log(GPR_INFO, "OpenSSL callback has already been set.");
}
is broken. Does it need an explanation or it is obvious?
> I have not done any research on them being not thread-safe so if you have
reference literature on it, that'd be good to see.
Library init functions typically aren't threadsafe.
> Still as the calls are idempotent, this thread synchronization is an
easy problem.
If you know every detail of every call -- yes, you can add synchronization
in all spots. Problem is that when you use 3rdparty lib that uses grpc
under the cover (or it uses other lib that uses grpc) -- I bet $50 it is
not going to be done correctly.
On Tue, Apr 24, 2018 at 8:17 PM, Arpit Baldeva <abald...@gmail.com> wrote:
> Your original remark said - there is still a race here with other
> thread(s) un-initializing OpenSSL (*talking about un-initializing*)
> Now you are saying - that is not what I meant by my remark --
> SSL-library-init functions are not threadsafe, afaik. (*talking about
> initializing*)
>
> My response was for the former. The original problem I had was that grpc
> would internally trounce my settings and I can not tell everyone else in my
> setting to wait for some unknown time when grpc would trounce those
> settings in some internal thread that I don't know about and only use
> OpenSSL after it. Now, it's an easier problem that I just need to make sure
> the OpenSSL is ready to use before I call grpc_init. As grpc does not do
> any shutdown on OpenSSL, I can ensure that my application is only cleaning
> it up after I am done with grpc.
>
> As for your latest remark that SSL-library-init functions are not
> threadsafe, they could also be wrapped inside the callback check. I have
> not done any research on them being not thread-safe so if you have
> reference literature on it, that'd be good to see. Still as the calls are
> idempotent, this thread synchronization is an easy problem.
>
>
> On Tue, Apr 24, 2018 at 5:45 PM, Michael Kilburn <crusader.m...@gmail.com>
> wrote:
>
>> Well, there is no official way to fully uninitialize OpenSSL. Also, that
>> is not what I meant by my remark -- SSL-library-init functions are not
>> threadsafe, afaik. I.e. calling them from different threads at the same
>> time is unsafe. And it is not easy to guarantee that this won't happen if
>> grpc is doing it under the covers.
>>
>> On Mon, Apr 23, 2018 at 10:43 PM, Arpit Baldeva <abald...@gmail.com>
>> wrote:
>>
>>> Grpc does not un-initialize OpenSSL. If you have other thread that is
>>> un-initing it, you can easily coordinate that after grpc shutdown. The
>>> patch is only ensuring that it is not overriding application settings.
>>> While the cleanest solution is to provide an explicit flag, the solution in
>>> the patch works as long as the user knows what’s happening behind the
>>> scenes.
>>>
>>> Sent from my iPhone
>>>
>>> On Apr 23, 2018, at 8:07 PM, 'Jiangtao Li' via grpc.io <
>>> grpc-io@googlegroups.com> wrote:
>>>
>>> Initializing OpenSSL outside gRPC is not practical in many situations.
>>> Our internal expert says the race condition only exist for OpenSSL 1.0.x
>>> and 0.9.x. We are on best effort to support older versions of OpenSSL,
>>> while our main support is BoringSSL and OpenSSL 1.1.
>>>
>>> Thanks,
>>> Jiangtao
>>>
>>>
>>> On Mon, Apr 23, 2018 at 7:30 PM CM <crusader.m...@gmail.com> wrote:
>>>
>>>> This change didn't really fix the problem -- there is still a race here
>>>> with other thread(s) un-initializing OpenSSL. Old problem, the only correct
>>>> solution is to leave OpenSSL initialization to user.
>>>>
>>>> On Monday, April 23, 2018 at 4:18:42 PM UTC-5, Jiangtao Li wrote:
>>>>>
>>>>> Good to know. Once the patch approved and merged. It will be in next
>>>>> grpc release.
>>>>>
>>>>>
>>>>> Thanks,
>>>>> Jiangtao
>>>>>
>>>>>
>>>>> On Mon, Apr 23, 2018 at 2:10 PM Arpit Baldeva <abal...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Thanks for the patch. It did fix the issue!
>>>>>>
>>>>>> What release version are you targeting?
>>>>>>
>>>>>> Thanks.
>>>>>>
>>>>>> On Friday, April 20, 2018 at 2:12:05 PM UTC-7, jian...@google.com
>>>>>> wrote:
>>>>>>>
>>>>>>> Arpit,
>>>>>>>
>>>>>>> Could you please patch https://github.com/grpc/grpc/pull/15143 and
>>>>>>> see whether it solves your problem.
>>>>>>>
>>>>>>> On Friday, April 20, 2018 at 9:30:34 AM UTC-7, Arpit Baldeva wrote:
>>>>>>>>
>>>>>>>> I am on 1.0.2k so yeah it is a problem on that version.
>>>>>>>>
>>>>>>>> I think the simplest fix is what I mentioned in last email - grpc
>>>>>>>> init_openssl implementation can check if locking callback already
>>>>>>>> exists by
>>>>>>>> calling CRYPTO_get_id_callback and if so, skip putting it's own locks.
>>>>>>>> The
>>>>>>>> application can ensure that it has the right initialization in place
>>>>>>>> before
>>>>>>>> doing anything with grpc.
>>>>>>>>
>>>>>>>> Thanks.
>>>>>>>>
>>>>>>>>
>>>>>>>> On Thursday, April 19, 2018 at 10:58:21 AM UTC-7, Jiangtao Li wrote:
>>>>>>>>>
>>>>>>>>> Which version of OpenSSL are you using? Or you are using
>>>>>>>>> BoringSSL? OpenSSL 1.1 or BoringSSL does not have such problems on
>>>>>>>>> OpenSSL
>>>>>>>>> init.
>>>>>>>>>
>>>>>>>>> For OpenSSL 1.0x, it is a valid concern. Let me check what is the
>>>>>>>>> best way to resolve this issue (pass a compiler flag, environment
>>>>>>>>> variable,
>>>>>>>>> or some API changes).
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Jiangtao
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Wed, Apr 18, 2018 at 7:34 PM Arpit Baldeva <abal...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> To explain further, a library such as grpc intializing global
>>>>>>>>>> settings of another library is not a good practice. Looking at
>>>>>>>>>> libcurl (
>>>>>>>>>> https://curl.haxx.se/libcurl/c/curl_global_init.html) ,
>>>>>>>>>> PostgreSQL(https://www.postgresql.org/docs/9.1/static/libpq-
>>>>>>>>>> ssl.html ) (and may be there are more libraries), these take an
>>>>>>>>>> initialization flag that to decide whether OpenSSL should be
>>>>>>>>>> initialized
>>>>>>>>>> internally or not.
>>>>>>>>>>
>>>>>>>>>> Actually, looking at https://stackoverflow.com/ques
>>>>>>>>>> tions/21874152/openssl-thread-safety-callback-function-regis
>>>>>>>>>> tration-with-both-direct-call-and-i , the simplest solution here
>>>>>>>>>> is for grpc to check if locking callback already exists by calling
>>>>>>>>>> CRYPTO_get_id_callback and if so, skip putting it's own locks. Rest
>>>>>>>>>> of the
>>>>>>>>>> init functions in OpenSSL are idempotent and hence can be called
>>>>>>>>>> multiple
>>>>>>>>>> times. And looks like grpc is not cleaning up OpenSSL so we are fine
>>>>>>>>>> there
>>>>>>>>>> too (looks like clean up calls are getting no-op in future -
>>>>>>>>>> https://stackoverflow.com/questions/35802643/will-ignoring-t
>>>>>>>>>> o-call-openssl-evp-cleanup-result-in-serious-flaws-or-memory-leak
>>>>>>>>>> ) .
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Wednesday, April 18, 2018 at 4:09:01 PM UTC-7, Arpit Baldeva
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Again, I am not sure why you are focusing on the race
>>>>>>>>>>> condition. Race condition is not the problem. Fact that grpc
>>>>>>>>>>> decides to
>>>>>>>>>>> put some global callbacks on the OpenSSL is the problem. It should
>>>>>>>>>>> not do
>>>>>>>>>>> that. Why can't this be made optional via a compile time flag or
>>>>>>>>>>> some run
>>>>>>>>>>> time initialization parameter?
>>>>>>>>>>>
>>>>>>>>>>> On Wednesday, April 18, 2018 at 3:52:47 PM UTC-7, Jiangtao Li
>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> I don't see a convenient way to do in gRPC. SSL init can be
>>>>>>>>>>>> called multiple times, as long as it is not called in the same
>>>>>>>>>>>> time. It
>>>>>>>>>>>> seems that the best way to address is adding mutex in your
>>>>>>>>>>>> application to
>>>>>>>>>>>> make sure SSL init is not called simultaneously.
>>>>>>>>>>>>
>>>>>>>>>>>> On Wed, Apr 18, 2018 at 3:28 PM Arpit Baldeva <
>>>>>>>>>>>> abal...@gmail.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Yes, there are two parallel threads that do this at the same
>>>>>>>>>>>>> time. What I was noticing is that at application shutdown, my
>>>>>>>>>>>>> application
>>>>>>>>>>>>> complained about the lock I handed over to OpenSSL not being
>>>>>>>>>>>>> unlocked.
>>>>>>>>>>>>> Looking into it, I realized that during initialization, OpenSSL
>>>>>>>>>>>>> grabbed a
>>>>>>>>>>>>> lock from my callback, during this time, grpc replaced the locks
>>>>>>>>>>>>> and
>>>>>>>>>>>>> OpenSSL unlock call got re-routed. But again, this is just a side
>>>>>>>>>>>>> effect.
>>>>>>>>>>>>> The real problem is simply the fact that grpc is setting some
>>>>>>>>>>>>> global
>>>>>>>>>>>>> callbacks on OpenSSL that should best be left to the application.
>>>>>>>>>>>>> I
>>>>>>>>>>>>> understand that this is probably done to make it easy for the grpc
>>>>>>>>>>>>> integrators but there are applications such as mine where there
>>>>>>>>>>>>> are other
>>>>>>>>>>>>> usage of OpenSSL and I'd rather manage the global settings myself
>>>>>>>>>>>>> than rely
>>>>>>>>>>>>> on grpc to do it.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wednesday, April 18, 2018 at 3:05:05 PM UTC-7,
>>>>>>>>>>>>> jian...@google.com wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Could you describe the race condition in details?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> In gRPC ssl_transport_security, init_openssl() is only
>>>>>>>>>>>>>> called when ssl transport security is needed. In your
>>>>>>>>>>>>>> application, you are
>>>>>>>>>>>>>> calling SSL_library_init() in a separate thread in parallel?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Wednesday, April 18, 2018 at 2:46:28 PM UTC-7, Arpit
>>>>>>>>>>>>>> Baldeva wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I am using grpc-1.10.0 and it has that code. Looking at the
>>>>>>>>>>>>>>> latest master, it still has that code -
>>>>>>>>>>>>>>> https://github.com/grpc/grpc/blob/master/src/core/tsi/ssl_
>>>>>>>>>>>>>>> transport_security.cc - see the init_openssl function.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> The problem is not just that grpc_init initialized the
>>>>>>>>>>>>>>> OpenSSL. The problem is really that grpc is initializing
>>>>>>>>>>>>>>> OpenSSL internally
>>>>>>>>>>>>>>> and it can trounce the application settings. Grpc should let
>>>>>>>>>>>>>>> application
>>>>>>>>>>>>>>> choose if they want it to handle the OpenSSL initialization or
>>>>>>>>>>>>>>> not.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> On Wednesday, April 18, 2018 at 12:25:58 PM UTC-7,
>>>>>>>>>>>>>>> jian...@google.com wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> Hi Arpit,
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> grpc_init initializes OpenSSL for a short period (~2 days)
>>>>>>>>>>>>>>>> and the code was later removed. Do you still the problem, if
>>>>>>>>>>>>>>>> you fetch the
>>>>>>>>>>>>>>>> latest master?
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> On Monday, April 16, 2018 at 2:22:32 PM UTC-7, Arpit
>>>>>>>>>>>>>>>> Baldeva wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Hi,
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> I recently pinned down a sporadic race condition in my
>>>>>>>>>>>>>>>>> application due to grpc intializing OpenSSL internally. The
>>>>>>>>>>>>>>>>> problem is that
>>>>>>>>>>>>>>>>> OpenSSL has some global callbacks that grpc is trying to
>>>>>>>>>>>>>>>>> initialize on it's
>>>>>>>>>>>>>>>>> own without the authorization of the application. The problem
>>>>>>>>>>>>>>>>> is in the
>>>>>>>>>>>>>>>>> init_openssl call in ssl_transport_security.cc which
>>>>>>>>>>>>>>>>> trounces similar calls from the application. The situation is
>>>>>>>>>>>>>>>>> made
>>>>>>>>>>>>>>>>> worse(race condition) if some application threads already
>>>>>>>>>>>>>>>>> have locks
>>>>>>>>>>>>>>>>> acquired via previous calls and then grpc changes the stuff
>>>>>>>>>>>>>>>>> underneath
>>>>>>>>>>>>>>>>> before the locks are released (hence the race condition).
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> My application has usage of OpenSSL in a wider context
>>>>>>>>>>>>>>>>> than just grpc. I get the point that grpc is wrapping this up
>>>>>>>>>>>>>>>>> to make it
>>>>>>>>>>>>>>>>> easier for standing up an application with grpc but I think
>>>>>>>>>>>>>>>>> such type of
>>>>>>>>>>>>>>>>> things should be accompanied by a compile/run time option
>>>>>>>>>>>>>>>>> supplied at
>>>>>>>>>>>>>>>>> grpc_init that let's application decide the right owner. I am
>>>>>>>>>>>>>>>>> fine with the
>>>>>>>>>>>>>>>>> option defaulting to grpc initializing the OpenSSL internally.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Thoughts?
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> Thanks.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> --
>>>>>>>>>>>>> You received this message because you are subscribed to a
>>>>>>>>>>>>> topic in the Google Groups "grpc.io" group.
>>>>>>>>>>>>> To unsubscribe from this topic, visit
>>>>>>>>>>>>> https://groups.google.com/d/topic/grpc-io/Z48vpXRnJaw/unsubs
>>>>>>>>>>>>> cribe.
>>>>>>>>>>>>> To unsubscribe from this group and all its topics, send an
>>>>>>>>>>>>> email to grpc-io+u...@googlegroups.com.
>>>>>>>>>>>>> To post to this group, send email to grp...@googlegroups.com.
>>>>>>>>>>>>> Visit this group at https://groups.google.com/group/grpc-io.
>>>>>>>>>>>>> To view this discussion on the web visit
>>>>>>>>>>>>> https://groups.google.com/d/msgid/grpc-io/3df13436-d771-4450
>>>>>>>>>>>>> -85c3-d2683b08e976%40googlegroups.com
>>>>>>>>>>>>> <https://groups.google.com/d/msgid/grpc-io/3df13436-d771-4450-85c3-d2683b08e976%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>>>>>>>>>>> .
>>>>>>>>>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>>>>>>>>>
>>>>>>>>>>>> --
>>>>>>>>>> You received this message because you are subscribed to a topic
>>>>>>>>>> in the Google Groups "grpc.io" group.
>>>>>>>>>> To unsubscribe from this topic, visit
>>>>>>>>>> https://groups.google.com/d/topic/grpc-io/Z48vpXRnJaw/unsubscribe
>>>>>>>>>> .
>>>>>>>>>> To unsubscribe from this group and all its topics, send an email
>>>>>>>>>> to grpc-io+u...@googlegroups.com.
>>>>>>>>>> To post to this group, send email to grp...@googlegroups.com.
>>>>>>>>>> Visit this group at https://groups.google.com/group/grpc-io.
>>>>>>>>>> To view this discussion on the web visit
>>>>>>>>>> https://groups.google.com/d/msgid/grpc-io/8821745f-9b23-4e3e
>>>>>>>>>> -b4cf-b88bce513b4b%40googlegroups.com
>>>>>>>>>> <https://groups.google.com/d/msgid/grpc-io/8821745f-9b23-4e3e-b4cf-b88bce513b4b%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>>>>>>>> .
>>>>>>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>>>>>>
>>>>>>>>> --
>>>>>> You received this message because you are subscribed to a topic in
>>>>>> the Google Groups "grpc.io" group.
>>>>>> To unsubscribe from this topic, visit https://groups.google.com/d/to
>>>>>> pic/grpc-io/Z48vpXRnJaw/unsubscribe.
>>>>>> To unsubscribe from this group and all its topics, send an email to
>>>>>> grpc-io+u...@googlegroups.com.
>>>>>> To post to this group, send email to grp...@googlegroups.com.
>>>>>> Visit this group at https://groups.google.com/group/grpc-io.
>>>>>> To view this discussion on the web visit
>>>>>> https://groups.google.com/d/msgid/grpc-io/d178eff0-8225-4a0b
>>>>>> -b63f-ac15afe425e3%40googlegroups.com
>>>>>> <https://groups.google.com/d/msgid/grpc-io/d178eff0-8225-4a0b-b63f-ac15afe425e3%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>>>> .
>>>>>> For more options, visit https://groups.google.com/d/optout.
>>>>>>
>>>>> --
>>>> You received this message because you are subscribed to a topic in the
>>>> Google Groups "grpc.io" group.
>>>> To unsubscribe from this topic, visit https://groups.google.com/d/to
>>>> pic/grpc-io/Z48vpXRnJaw/unsubscribe.
>>>> To unsubscribe from this group and all its topics, send an email to
>>>> grpc-io+unsubscr...@googlegroups.com.
>>>> To post to this group, send email to grpc-io@googlegroups.com.
>>>> Visit this group at https://groups.google.com/group/grpc-io.
>>>> To view this discussion on the web visit https://groups.google.com/d/ms
>>>> gid/grpc-io/221ffdb6-aa2c-457d-8772-32ace46173dc%40googlegroups.com
>>>> <https://groups.google.com/d/msgid/grpc-io/221ffdb6-aa2c-457d-8772-32ace46173dc%40googlegroups.com?utm_medium=email&utm_source=footer>
>>>> .
>>>> For more options, visit https://groups.google.com/d/optout.
>>>>
>>> --
>>> You received this message because you are subscribed to a topic in the
>>> Google Groups "grpc.io" group.
>>> To unsubscribe from this topic, visit https://groups.google.com/d/to
>>> pic/grpc-io/Z48vpXRnJaw/unsubscribe.
>>> To unsubscribe from this group and all its topics, send an email to
>>> grpc-io+unsubscr...@googlegroups.com.
>>> To post to this group, send email to grpc-io@googlegroups.com.
>>> Visit this group at https://groups.google.com/group/grpc-io.
>>> To view this discussion on the web visit https://groups.google.com/d/ms
>>> gid/grpc-io/CACcm8_i7f6HdGG3G72p_FN2UqwyUzaDyDVKGE3oNP1Xbi9G
>>> SwA%40mail.gmail.com
>>> <https://groups.google.com/d/msgid/grpc-io/CACcm8_i7f6HdGG3G72p_FN2UqwyUzaDyDVKGE3oNP1Xbi9GSwA%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>> .
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>>
>>
>>
>> --
>> Sincerely yours,
>> Michael.
>>
>
>
--
Sincerely yours,
Michael.
--
You received this message because you are subscribed to the Google Groups
"grpc.io" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to grpc-io+unsubscr...@googlegroups.com.
To post to this group, send email to grpc-io@googlegroups.com.
Visit this group at https://groups.google.com/group/grpc-io.
To view this discussion on the web visit
https://groups.google.com/d/msgid/grpc-io/CA%2BnLVP6xcFp0McF1ADJ6woCBLV_BSPwCkiQxpBuzKW5WyF0yNQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
