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

MSVC crash with '-DMEMDEBUG_LOG_SYNC' #828

Closed
gvanem opened this Issue May 25, 2016 · 6 comments

Comments

Projects
None yet
3 participants
@gvanem
Member

gvanem commented May 25, 2016

Building libcurl with -DCURLDEBUG and -DMEMDEBUG_LOG_SYNC for me causes crash in lib/memdebug.c inside curl_memdebug():

    setvbuf(logfile, (char *)NULL, _IOLBF, 0);

I.e. Dr. Watson triggers a run-time check followed by a crash. But this seems to work as intended:

    setvbuf(logfile, (char *)NULL, _IONBF, 0);

Anybody who can confirm this?

This seems to be the same as this Stackoverflow issue. I'm using MSVC-2015 (cl 19.00.23506 for x86) on Win-10.

@dfandrich

This comment has been minimized.

Show comment
Hide comment
@dfandrich

dfandrich May 25, 2016

Collaborator

I'm guessing the issue has to do with the size 0 passed in when line buffering; does it work if you pass in 1024 for example? In any case, unbuffering this stream is just as good (if not actually a better) of a solution in this case.

Collaborator

dfandrich commented May 25, 2016

I'm guessing the issue has to do with the size 0 passed in when line buffering; does it work if you pass in 1024 for example? In any case, unbuffering this stream is just as good (if not actually a better) of a solution in this case.

@gvanem

This comment has been minimized.

Show comment
Hide comment
@gvanem

gvanem May 25, 2016

Member

does it work if you pass in 1024 for example?

It does.

Not sure what you mean by unbuffering this stream, but trying with this:

--- a/memdebug.c 2016-05-01 22:05:17
+++ b/memdebug.c 2016-05-25 19:41:33
@@ -118,9 +118,18 @@
     else
       logfile = stderr;
 #ifdef MEMDEBUG_LOG_SYNC
-    /* Flush the log file after every line so the log isn't lost in a crash */
+    /* Flush the log file after every line so the log isn't lost in a crash.
+     * For Win32, the mode *must* be _IONBF. Otherwise Dr. Watson triggers a
+     * run-time check followed by a crash.
+     *   Refs: https://msdn.microsoft.com/en-us/library/86cebhfs.aspx
+     *         http://stackoverflow.com/questions/15972550/what-is-the-use-of-setvbuf-in-an-applicationexecutable
+     */
+#if defined(WIN32)
+    setvbuf(logfile, (char *)NULL, _IONBF, 0);
+#else
     setvbuf(logfile, (char *)NULL, _IOLBF, 0);
 #endif
+#endif
   }
 }

seems okay.

Member

gvanem commented May 25, 2016

does it work if you pass in 1024 for example?

It does.

Not sure what you mean by unbuffering this stream, but trying with this:

--- a/memdebug.c 2016-05-01 22:05:17
+++ b/memdebug.c 2016-05-25 19:41:33
@@ -118,9 +118,18 @@
     else
       logfile = stderr;
 #ifdef MEMDEBUG_LOG_SYNC
-    /* Flush the log file after every line so the log isn't lost in a crash */
+    /* Flush the log file after every line so the log isn't lost in a crash.
+     * For Win32, the mode *must* be _IONBF. Otherwise Dr. Watson triggers a
+     * run-time check followed by a crash.
+     *   Refs: https://msdn.microsoft.com/en-us/library/86cebhfs.aspx
+     *         http://stackoverflow.com/questions/15972550/what-is-the-use-of-setvbuf-in-an-applicationexecutable
+     */
+#if defined(WIN32)
+    setvbuf(logfile, (char *)NULL, _IONBF, 0);
+#else
     setvbuf(logfile, (char *)NULL, _IOLBF, 0);
 #endif
+#endif
   }
 }

seems okay.

@dfandrich

This comment has been minimized.

Show comment
Hide comment
@dfandrich

dfandrich May 25, 2016

Collaborator

I was talking about using _IONBF unconditionally, which sets the stream unbuffered. Don't bother making it conditional, just use _IONBF unconditionally, either the way you have it or the slightly simpler setbuf(logfile, (char*)NULL);

Collaborator

dfandrich commented May 25, 2016

I was talking about using _IONBF unconditionally, which sets the stream unbuffered. Don't bother making it conditional, just use _IONBF unconditionally, either the way you have it or the slightly simpler setbuf(logfile, (char*)NULL);

@gvanem

This comment has been minimized.

Show comment
Hide comment
@gvanem

gvanem May 25, 2016

Member

Ah, yes this works:

--- a/memdebug.c 2016-05-01 22:05:17
+++ b(memdebug.c 2016-05-25 23:10:57
@@ -119,7 +119,7 @@
       logfile = stderr;
 #ifdef MEMDEBUG_LOG_SYNC
     /* Flush the log file after every line so the log isn't lost in a crash */
-    setvbuf(logfile, (char *)NULL, _IOLBF, 0);
+    setbuf(logfile, (char *)NULL);
 #endif
   }
 }
Member

gvanem commented May 25, 2016

Ah, yes this works:

--- a/memdebug.c 2016-05-01 22:05:17
+++ b(memdebug.c 2016-05-25 23:10:57
@@ -119,7 +119,7 @@
       logfile = stderr;
 #ifdef MEMDEBUG_LOG_SYNC
     /* Flush the log file after every line so the log isn't lost in a crash */
-    setvbuf(logfile, (char *)NULL, _IOLBF, 0);
+    setbuf(logfile, (char *)NULL);
 #endif
   }
 }

@bagder bagder closed this in 9a15935 May 30, 2016

@bagder

This comment has been minimized.

Show comment
Hide comment
@bagder

bagder May 30, 2016

Member

Thanks!

Member

bagder commented May 30, 2016

Thanks!

@gvanem

This comment has been minimized.

Show comment
Hide comment
@gvanem

gvanem Jul 6, 2017

Member

This issue seems to have surfaced once again. Now in C-ares when CURLDEBUG is in effect.
Turned out to be an user-error; I set %CARES_MEMDEBUG to a non-existing directory/file. I fixed it simply by:

--- a/memdebug.c 2017-05-25 14:03:18
+++ b/memdebug.c 2017-07-06 18:03:24
@@ -115,6 +115,7 @@
       logfile = stderr;
 #ifdef MEMDEBUG_LOG_SYNC
     /* Flush the log file after every line so the log isn't lost in a crash */
+    if (logfile)
        setbuf(logfile, (char *)NULL);
 #endif
   }
Member

gvanem commented Jul 6, 2017

This issue seems to have surfaced once again. Now in C-ares when CURLDEBUG is in effect.
Turned out to be an user-error; I set %CARES_MEMDEBUG to a non-existing directory/file. I fixed it simply by:

--- a/memdebug.c 2017-05-25 14:03:18
+++ b/memdebug.c 2017-07-06 18:03:24
@@ -115,6 +115,7 @@
       logfile = stderr;
 #ifdef MEMDEBUG_LOG_SYNC
     /* Flush the log file after every line so the log isn't lost in a crash */
+    if (logfile)
        setbuf(logfile, (char *)NULL);
 #endif
   }

bagder added a commit that referenced this issue Jul 6, 2017

@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018

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