-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Call curl_global_init in _init for all plugins #526
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This is a shot in the dark in trying to address collectd#513. By calling this from an init() callback, I hope to be initializing the curl and gcrypt libraries before collectd becomes multi-threaded, avoiding the problems described in the issue.
Need to call curl_global_init() or curl_easy_init() during init for plugins when we're still running single threaded. This updates the remaining ones
LGTM, but the apache plugin might create merge conflicts with the collectd-5.4 branch. I'll take a look later today. |
It's a cherry-pick, so git merging should handle it reasonably well |
@katzj working fine for me here |
This patch should probably be applied to bind.c (and any others that make use of curl) as well. |
I went through and ensured that all of the plugins using libcurl are calling the curl init from the plugin's init. This pull request takes care of the ones which weren't already doing so |
So, at the time I had applied these patches to a backport of collectd 5.4.0 for Debian Squeeze and was still winding up with curl/apache related gnutls/gcrypt SIGABRTs. I had though it was because bind was also using curl but neglecting to perform the ssl initiating (curl_global_init()), but it didn't actually help (probably because it just calls curl_easy_init() which in turn calls curl_global_init() if it hadn't already been done). After much digging I later discovered that for that old version of gnutls (2.8.6), I had to call gcry_control(GCRYCTL_SET_THREAD_CBS, ...) from within collectd.c while it was still single threaded. Seems later versions of gnutls (at least 2.12.20 from Debian Wheezy) handle this for you. Anyways, sorry for the confusion. I include it here not as request to fix, but just an FYI in case any one else runs into this (hopefully not). Thanks, |
Thanks @katzj ! So according to the comments, there's a pretty good consensus on your patch :) I cherry-picked it onto the collectd-4.10 branch as 66b400a. It will eventually get merged in the collectd-5.3, collectd-5.4 and master branches. FWIW, it causes no conflict when merged onto these branches (in their state as of today). |
I haven't seen it, but #513 could definitely manifest for other plugins as well. This makes it so that all plugins are either using curl_global_init() or curl_easy_init() from their init function.