Skip to content

Possible improper locking issues #7926

Closed

Description

@ryancaicse

HI, in this function, it is possible that the lock lock (Line 71) is not released properly because the execution does not enter the while(j < num_urls)? Also, it also seems that the lock lock could be released multiple times in the while loop at Line 97.

Thanks for checking.

void *pull_one_url(void *NaN)
{
/* Stop threads from entering unless j is incremented */
pthread_mutex_lock(&lock);
while(j < num_urls) {
CURL *curl;
gchar *http;
printf("j = %d\n", j);
http =
g_strdup_printf("xoap.weather.com/weather/local/%s?cc=*&dayf=5&unit=i\n",
urls[j]);
printf("http %s", http);
curl = curl_easy_init();
if(curl) {
FILE *outfile = fopen(urls[j], "wb");
/* Set the URL and transfer type */
curl_easy_setopt(curl, CURLOPT_URL, http);
/* Write to the file */
curl_easy_setopt(curl, CURLOPT_WRITEDATA, outfile);
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, write_file);
j++; /* critical line */
pthread_mutex_unlock(&lock);
curl_easy_perform(curl);
fclose(outfile);
printf("fclose\n");
curl_easy_cleanup(curl);
}
g_free(http);
/* Adds more latency, testing the mutex.*/
sleep(1);
} /* end while */
return NULL;
}

Activity

  1. bagder commented on Oct 30, 2021

    @bagder
    Member

    It does indeed look very wrong. You up for writing a pull request and fix it?

  2. ryancaicse commented on Oct 30, 2021

    @ryancaicse
    Author

    Sorry, I don't know which is the best way to fix them due to being unaware of the shared data protected by the lock. But, here is my try. Is this enough to fix this issue?

     void *pull_one_url(void *NaN) 
     { 
       /* Stop threads from entering unless j is incremented */ 
       pthread_mutex_lock(&lock); 
       while(j < num_urls) { 
         ...;
         if(curl) { 
          ...;
           pthread_mutex_unlock(&lock); 
      
           ...;
           pthread_mutex_lock(&lock); // new added
         } 
         ...;
       } /* end while */ 
        pthread_mutex_unlock(&lock); // new added
       return NULL; 
     }
    
  3. added a commit that references this issue on Oct 31, 2021
    876e9da
  4. bagder commented on Oct 31, 2021

    @bagder
    Member

    I think it needs more than that. See #7931 for my proposed fix.

  5. ryancaicse commented on Nov 1, 2021

    @ryancaicse
    Author

    OK, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions