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 <[email protected]> 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, [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 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/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 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/CACcm8_jHvN%3DHEK1yz55R-69_BTKwmF0uRo6B9afu5mHDrNeZgQ%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
