darcs

Issue 472 bad interaction between withCString and libcurl

Title bad interaction between withCString and libcurl
Priority bug Status resolved
Milestone Resolved in
Superseder Nosy List darcs-devel, dmitry.kurochkin, jcpetruzza, kowey, thorkilnaur, tommy
Assigned To
Topics

Created on 2007-06-15.20:15:51 by dgorin, last changed 2009-10-24.00:45:55 by admin.

Files
File name Uploaded Type Edit Remove
safe_curl_easy_setopt.patch dgorin, 2007-06-22.15:27:47 application/octet-stream
t.hs dgorin, 2007-06-15.20:15:39 application/octet-stream
test_curl.c dgorin, 2007-06-15.20:15:39 application/octet-stream
test_curl.h dgorin, 2007-06-15.20:15:39 application/octet-stream
unnamed dgorin, 2007-06-15.20:15:39 text/plain
unnamed droundy, 2007-06-17.14:53:45 text/plain
Messages
msg1718 (view) Author: jcpetruzza Date: 2007-06-15.20:15:40
Hi

I've been having some non-deterministic misbehaviour when interacting  
with repositories via http. For example, running several times "darcs  
send -o my_patch http://some/repo" would give alternatively one of  
this results:

- "Can't understand repository"
- Darcs tries to send ALL the patches in my local repo (most came  
from the remote repo)
- Darcs behaves properly

I tracked down the error and I understand it boils down to this: non- 
deterministacally, libcurl is returning, instead of a 404 error, the  
webserver-generated document containing the 404 error (i.e. it is  
behaving as curl without the -f flag). Depending the moment in the  
execution of darcs where this misbehaviour occurs, I get the  
different results above (when it does not occur, darcs works fine).

Now, why is this happening? I think the reason is this: in Curl.hs,  
there is this function:

copyUrl u f cache =
   withCString darcs_version $ \vstr ->
   withCString u $ \ustr ->
   withCString f $ \fstr -> do
   ppwd <- getProxyUserPwd
   withCString ppwd $ \pstr -> do
   err <- get_curl vstr pstr fstr ustr (cachableToInt cache)
   when (err /= 0) $ fail $ ...

It calls get_curl, defined in hscurl.c, which in turn uses a _static_  
variable "c", that is created using curl_easy_init and managed with  
curl_easy_setopt.

However, the manpage for curl_easy_setopt states:

"Strings passed to libcurl as 'char *' arguments, will not be copied  
by the library. Instead you should keep them available until libcurl  
no longer needs them. Failing to do so will cause very odd behavior  
or even crashes. libcurl will need them until you call  
curl_easy_cleanup(3) or you set the same option again to use a  
different pointer."

while the documentation for withCString says:

"the memory is freed when the subcomputation terminates (either  
normally or via an exception), so the pointer to the temporary  
storage must not be used after this."

I was able to come up with a small test case that non- 
deterministically fails on my computer and I can confirm that, by not  
reusing the curl handler, the misbehaviour vanishes (incidentally, it  
also runs much faster). I'm attaching it but, due to the nature of  
this error, I'm not confident it will also fail elsewhere.

For the record, I had this issue using darcs 1.0.8 (installed through  
macports) and darcs 1.0.9 (compiled from sources). Curl version is  
7.16.1 (also from macports), and I'm running an Intel-based Mac,  
running OS X 10.4.9

Thanks
Daniel
Attachments
msg1725 (view) Author: droundy Date: 2007-06-17.14:53:45
> I was able to come up with a small test case that non- 
> deterministically fails on my computer and I can confirm that, by not  
> reusing the curl handler, the misbehaviour vanishes (incidentally, it  
> also runs much faster). I'm attaching it but, due to the nature of  
> this error, I'm not confident it will also fail elsewhere.

Wow, thanks for hunting this down!  Willing to submit a patch?
Attachments
msg1746 (view) Author: jcpetruzza Date: 2007-06-22.15:27:47
> Wow, thanks for hunting this down!  Willing to submit a patch?

sure, no problem. i'm attaching one here.

however, this still didn't solve my original issue.... but upgrading  
libcurl to 7.16.2 did!

i didn't look much further into this; for the time being i take it to  
be a (hopefully) fixed bug in libcurl.
Attachments
msg1821 (view) Author: kowey Date: 2007-07-14.20:37:46
Well, I'm pushing Daniel's patch to unstable.

As he points out, this doesn't solve the issue, although upgrading libcurl
seemed to do the job.  I'm going to close this bug.  If future gremlims appear
and upgrading libcurl does not help, we'll have to take a closer look.
History
Date User Action Args
2007-06-15 20:15:51dgorincreate
2007-06-17 14:53:46droundysetfiles: + unnamed
nosy: droundy, tommy, beschmi, kowey, dgorin
status: unread -> unknown
messages: + msg1725
2007-06-22 15:27:49dgorinsetfiles: + safe_curl_easy_setopt.patch
messages: + msg1746
2007-07-14 20:37:56koweysetstatus: unknown -> resolved-in-unstable
messages: + msg1821
2007-07-31 17:59:51koweysetstatus: resolved-in-unstable -> resolved-in-stable
2008-09-16 21:31:06adminsetstatus: resolved-in-stable -> resolved
nosy: + dagit
2009-08-06 17:32:50adminsetnosy: + markstos, jast, Serware, dmitry.kurochkin, darcs-devel, zooko, mornfall, simon, thorkilnaur, - droundy, dgorin
2009-08-06 20:30:14adminsetnosy: - beschmi
2009-08-10 22:04:55adminsetnosy: + dgorin, - markstos, darcs-devel, zooko, jast, Serware, mornfall
2009-08-11 00:01:02adminsetnosy: - dagit
2009-08-25 17:47:37adminsetnosy: + darcs-devel, - simon
2009-08-27 14:11:49adminsetnosy: tommy, kowey, darcs-devel, dgorin, thorkilnaur, dmitry.kurochkin
2009-10-24 00:45:55adminsetnosy: + jcpetruzza, - dgorin