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 <abald...@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/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 <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/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/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+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/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 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/CACcm8_gs4%3DOE5NbUCsCLJjTStkwoJi7zLO%3D0W4iTS3ceQkRVAA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
