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/94d02c43-e6f9-4a74-8195-024ea4fe8001%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.
