search

Lua-cURL / Lua-cURLv3

Lua binding to libcurl
MIT License
275 stars 63 forks source link

implemented support for custom curl_global_init #169

Closed cryi closed 3 years ago

cryi commented 3 years ago

Hi,

I tried to resolve #121. Everything works as before and I verified that library is not initialized if LCURL_NO_INIT set. Manual initialization is possible with init function on both lcurl and lcurl.safe. Although I am not sure whether expose CURL_GLOBAL_DEFAULT and others directly on lcurl and lcurls.safe, so for now initialization mode has to be passed as number.

Although I am not able to verify custom initialization. I use only easy methods and easy calls curl_global_init on its own in case it was not initialized before. It would be great if you can test in more suitable scenario.

Fixes #121.

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.2%) to 63.193% when pulling 1541417cee1cc0f9dd6f0985d0ae9f5e5fdd1a58 on cryi:global_init into d27a1d3d0da31868be6f047242b447f29ba743bd on Lua-cURL:master.

coveralls commented 3 years ago

Coverage Status

Coverage decreased (-0.2%) to 63.193% when pulling 1541417cee1cc0f9dd6f0985d0ae9f5e5fdd1a58 on cryi:global_init into d27a1d3d0da31868be6f047242b447f29ba743bd on Lua-cURL:master.

moteus commented 3 years ago

This is not a solution for #121 There the main problem that host application init libcurl by itself and then run Lua code. So in this case Lua should not init curl at all.

cryi commented 3 years ago

Are you sure @moteus? Setting it from lua is just optional. As long as LCURL_NO_INIT is specified - check here. Lua wont initialize libcurl on its own.

I just added option to initialize it from lua manually on top of your requirement to disable initialization.

moteus commented 3 years ago

Sorry. You are right. I have only one (very small) concern. getenv is not thread safe function. But I do not see a better way to handle this rather than just ignoring this.

cryi commented 3 years ago

Well curl_global_init is not thread safe on its own. So in context of loading lcurl there should not be difference after merging.

getenv thread safety is mostly about the validity of pointer it returns (it may invalidated over time in multi threaded setup) but we do not care for its validity - we do not read anything from it. NULL value is our only concern here and only potential issue I can see, would be if someone tries to set LCURL_NO_INIT from background thread while loading lcurl on main or vice versa. We can mention that in docs.

But I believe that devs working in multithreaded lua realize that initialization has to be done before everything else and as such setting LCURL_NO_INIT has to be done before spawning multiple threads of lcurl or lua states containing lcurl.