2014/07/15 3:39、Daniel Stenberg <dan...@haxx.se> のメール:

> On Sun, 13 Jul 2014, yousuke.kim...@gmail.com wrote:
> 
>> This issue was reported in http://curl.haxx.se/mail/lib-2014-02/0184.html. 
>> Unfortunately the reporter didn't suggest his patch.
>> 
>> This issue is caused by locking CURL_LOCK_DATA_COOKIE with specified cookie 
>> files. In this case, CURL_LOCK_DATA_COOKIE is locked before executing 
>> "FLUSH", and then Curl_cookie_loadfiles locks CURL_LOCK_DATA_COOKIE. That is 
>> deadlock.
> 
> Thanks for the patch. My reading of it says it is incorrect though:
> 
> - for FLUSH you (still) take the lock before Curl_flush_cookies() is called
>   and then within Curl_flush_cookies() it locks the same mutex again.
> 
> Shouldn't the patch then just have those lines removed, like this?

Thank you for your patch.
I misunderstood your comment, I'm sorry.

Your patch is right, Curl_share_lock() and Curl_share_unlock() are not
necessary for Curl_flush_cookies().

Thanks,
Yousuke


> 
> From fcf80e75c39a54b8be53d7040fe8970c3b13a542 Mon Sep 17 00:00:00 2001
> From: Daniel Stenberg <dan...@haxx.se>
> Date: Mon, 14 Jul 2014 20:38:18 +0200
> Subject: [PATCH] cookie: avoid mutex deadlock
> 
> ... by removing the extra mutex locks around th call to
> Curl_flush_cookies() which takes care of the locking itself already.
> ---
> lib/url.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/url.c b/lib/url.c
> index 87ebe00..1d05975 100644
> --- a/lib/url.c
> +++ b/lib/url.c
> @@ -1167,22 +1167,24 @@ CURLcode Curl_setopt(struct SessionHandle *data, 
> CURLoption option,
>     argptr = va_arg(param, char *);
> 
>     if(argptr == NULL)
>       break;
> 
> -    Curl_share_lock(data, CURL_LOCK_DATA_COOKIE, CURL_LOCK_ACCESS_SINGLE);
> -
>     if(Curl_raw_equal(argptr, "ALL")) {
>       /* clear all cookies */
> +      Curl_share_lock(data, CURL_LOCK_DATA_COOKIE, CURL_LOCK_ACCESS_SINGLE);
>       Curl_cookie_clearall(data->cookies);
> +      Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE);
>     }
>     else if(Curl_raw_equal(argptr, "SESS")) {
>       /* clear session cookies */
> +      Curl_share_lock(data, CURL_LOCK_DATA_COOKIE, CURL_LOCK_ACCESS_SINGLE);
>       Curl_cookie_clearsess(data->cookies);
> +      Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE);
>     }
>     else if(Curl_raw_equal(argptr, "FLUSH")) {
> -      /* flush cookies to file */
> +      /* flush cookies to file, takes care of the locking */
>       Curl_flush_cookies(data, 0);
>     }
>     else {
>       if(!data->cookies)
>         /* if cookie engine was not running, activate it */
> @@ -1191,23 +1193,24 @@ CURLcode Curl_setopt(struct SessionHandle *data, 
> CURLoption option,
>       argptr = strdup(argptr);
>       if(!argptr) {
>         result = CURLE_OUT_OF_MEMORY;
>       }
>       else {
> +        Curl_share_lock(data, CURL_LOCK_DATA_COOKIE, 
> CURL_LOCK_ACCESS_SINGLE);
> 
>         if(checkprefix("Set-Cookie:", argptr))
>           /* HTTP Header format line */
>           Curl_cookie_add(data, data->cookies, TRUE, argptr + 11, NULL, NULL);
> 
>         else
>           /* Netscape format line */
>           Curl_cookie_add(data, data->cookies, FALSE, argptr, NULL, NULL);
> 
> +        Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE);
>         free(argptr);
>       }
>     }
> -    Curl_share_unlock(data, CURL_LOCK_DATA_COOKIE);
> 
>     break;
> #endif /* CURL_DISABLE_COOKIES */
> 
>   case CURLOPT_HTTPGET:
> -- 
> 2.0.1
> 
> 
> -- 
> 
> / daniel.haxx.se
> -------------------------------------------------------------------
> List admin: http://cool.haxx.se/list/listinfo/curl-library
> Etiquette:  http://curl.haxx.se/mail/etiquette.html


-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html

Reply via email to