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, [email protected] 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 <[email protected]> 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/questions/21874152/openssl-thread-safety-callback-function-registration-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-to-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 <[email protected]> >>>>>> 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, [email protected] >>>>>>> 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, >>>>>>>>> [email protected] 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/unsubscribe. >>>>>>> To unsubscribe from this group and all its topics, send an email to >>>>>>> [email protected]. >>>>>>> To post to this group, send email to [email protected]. >>>>>>> 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 >>>> [email protected]. >>>> To post to this group, send email to [email protected]. >>>> 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 the Google Groups "grpc.io" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. 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. For more options, visit https://groups.google.com/d/optout.
