Menu

#1362 Increment the initialized counter in curl_global_init_mem()

closed-fixed
5
2015-02-14
2014-04-17
Tristan
No

Hello,

I think there is an issue in easy.c, in the curl_global_init_mem() function
The initialized counter is not incremented if the libcurl is already initialized.
I'm currently doing two inits and two shutdowns, and here is the problem I met :

1) curl_global_init_mem(); // Actually initialize libcurl (initialized = 1)
2) curl_global_init_mem(); // Should do nothing (initialized stays to 1 instead of being incremented)
3) curl_global_cleanup(); // Actually cleanup libcurl because initialized = 1
4) curl_global_cleanup();

After step 3 the variables that were created using the custom malloc are released using the default free, which raises an assert by libc on non-matching heap :
Invalid address specified to RtlValidateHeap

I could fix that by modifying easy.c the following way:
@@ -300,8 +300,10 @@
return CURLE_FAILED_INIT;

   /* Already initialized, don't do it again */
-  if(initialized)
+  if(initialized) {
+    initialized++;
     return CURLE_OK;
+  }

   /* Call the actual init function first */
   code = curl_global_init(flags);

However, maybe there is a better fix than duplicating the increment from curl_global_init()?
I'm using Libcurl 7.34.0, and checked quickly for a fix in 7.36.0, but I didn't find it.

Discussion

  • Tristan

    Tristan - 2014-04-18

    Hello,

    After doing some more tests, the use case I described yesterday is actually not producing the libc assert. I thought that the cleanup set the default memory manager back, but it's not (which in fact seems ok to me).

    I guess the fix I suggested is still legit, but for information, here is the use case producing the assert :
    1) curl_global_init_mem() // initialization with custom memory manager
    2) curl_global_init_mem()
    3) curl_global_cleanup() // cleanup
    4) curl_multi_init() // initialization with default memory manager
    5) curl_global_init_mem()
    6) curl_global_cleanup() // cleanup
    7) curl_global_cleanup()

    I didn't know where the default memory managers were set back, and I just discovered the step 4 in my code and that it was calling curl_global_init().

    Sorry for the wrong use case from yesterday

     
  • Daniel Stenberg

    Daniel Stenberg - 2014-04-21

    If we increase the variable there, it will effectively tell curl_global_init() to not do anything which then would make it pointless to call it - and then some things won't be initialized at all.

    IOW: that's not a good fix...

     
  • Daniel Stenberg

    Daniel Stenberg - 2014-04-21
    • assigned_to: Daniel Stenberg
    • Priority: 2 --> 5
     
  • Tristan

    Tristan - 2014-04-22

    Hello Mr. Stenberg,

    Thank you for your answer.

    Indeed there should be a better fix than this one.
    However I'm not sure to understand your explanation.
    In the suggested fix, the initialized variable is only incremented when curl_global_init() will not be called, i.e. when initialized is already > 0. Everything should already be initialized correctly before reaching the updated code.

    Is it correct if I say that after each call to curl_global_init_mem(), the initialized variable should have its value incremented?

    The suggested fix is not very good since it would be better that the initialized variable would be updated only in curl_global_init() and curl_global_cleanup(), but is this fix wrong or just not good?

     
  • Daniel Stenberg

    Daniel Stenberg - 2014-04-22
    • status: open --> open-confirmed
     
  • Daniel Stenberg

    Daniel Stenberg - 2014-04-22

    Thanks for being patient and explaining. I agree with you now that this change is necessary to have curl_global_init_mem() work the same way as curl_global_init().

     
  • Daniel Stenberg

    Daniel Stenberg - 2014-04-22

    Thanks, this fix has now been pushed to git!

     
  • Daniel Stenberg

    Daniel Stenberg - 2014-04-22
    • status: open-confirmed --> closed-fixed