thread assert with 1.8.2

thread assert with 1.8.2

[Greg Troxel]

I just updated pkgsrc to 1.8.2, and on NetBSD-current/i386 make check
fails in the thread test with:

Running threads.test
guile: Error detected by libpthread: Unlocking unlocked mutex.
Detected by file "/usr/home/gdt/NetBSD-current/src/lib/libpthread/pthread_mutex.c", line 357, function "pthread_mutex_unlock".
See pthread(3) for information.
[1]   Abort trap (core dumped) ${dir}${tst}

The core has several threads, nad this is the one that hit the assert:

(gdb) bt
#0  0xbb9d6bdb in kill () from /usr/lib/libc.so.12
#1  0xbbaa3d9b in pthread__errorfunc () from /usr/lib/libpthread.so.0
#2  0xbbaa1d49 in pthread_mutex_unlock () from /usr/lib/libpthread.so.0
#3  0xbbb9875c in scm_leave_guile () from /usr/home/gdt/NetBSD-current/pkgsrc/lang/guile/work/guile-1.8.2/libguile/.libs/libguile.so.17
#4  0xbbb99c4e in on_thread_exit () from /usr/home/gdt/NetBSD-current/pkgsrc/lang/guile/work/guile-1.8.2/libguile/.libs/libguile.so.17
#5  0xbbaa0e1c in pthread__destroy_tsd () from /usr/lib/libpthread.so.0
#6  0xbbaa5119 in pthread_exit () from /usr/lib/libpthread.so.0
#7  0xbbaa5499 in pthread_setcancelstate () from /usr/lib/libpthread.so.0
#8  0xbb9f9370 in swapcontext () from /usr/lib/libc.so.12

Perhaps someone could try this on Linux with the right magic debug
variables to object to unlocking an unlocked mutex.  (On NetBSD, the
default behavior is to abort on any operation which the standard says is
undefined.)

On NetBSD 4.0ish, which has a different thread implementation, I get the
same assert.




_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: thread assert with 1.8.2

[Kevin Ryde]

Greg Troxel <gdt@ir.bbn.com> writes:
>
> Running threads.test
> guile: Error detected by libpthread: Unlocking unlocked mutex.
> Detected by file "/usr/home/gdt/NetBSD-current/src/lib/libpthread/pthread_mutex.c", line 357, function "pthread_mutex_unlock".

Sounds bad.

> Perhaps someone could try this on Linux with the right magic debug
> variables to object to unlocking an unlocked mutex.

For a gnu system pthread_mutex_unlock returns non-zero for a problem,
except it only notices problems if you use the "error checking" flavour
of mutexes (currently SCM_I_PTHREAD_MUTEX_INITIALIZER is the default
"fast" kind, which don't check).

I suppose a debug-build option to use the checking flavour and look at
the returns would be a good thing.  I guess the "fat_mutex" stuff at the
scheme level is supposed to take care of keeping scheme code out of
trouble though.


_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: thread assert with 1.8.2

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 752 bytes --]

Hi,

Greg Troxel <gdt@ir.bbn.com> writes:

> I just updated pkgsrc to 1.8.2, and on NetBSD-current/i386 make check
> fails in the thread test with:

Does this happen with previous 1.8 releases?

> Perhaps someone could try this on Linux with the right magic debug
> variables to object to unlocking an unlocked mutex.  (On NetBSD, the
> default behavior is to abort on any operation which the standard says is
> undefined.)

I tried the attached patch (which does what Kevin suggested) on
GNU/Linux, and recompiled with `-DSCM_USE_ERROR_CHECKING_MUTEXES=1'.

I then run the whole test-suite (on i686), without observing anything
special.  Running `threads.test' in a loop didn't trigger the error
either.

Further investigation is needed...

Ludovic.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Patch that adds support for error-checking mutexes --]
[-- Type: text/x-patch, Size: 1954 bytes --]

--- orig/libguile/pthread-threads.h
+++ mod/libguile/pthread-threads.h
@@ -43,13 +43,51 @@
 
 /* Mutexes
  */
-#define SCM_I_PTHREAD_MUTEX_INITIALIZER     PTHREAD_MUTEX_INITIALIZER
+#ifdef SCM_USE_ERROR_CHECKING_MUTEXES
+# if (!defined PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP) || (!defined __GNUC__)
+#  undef SCM_USE_ERROR_CHECKING_MUTEXES
+# endif
+#endif
+
+#ifdef SCM_USE_ERROR_CHECKING_MUTEXES
+# define SCM_I_PTHREAD_MUTEX_INITIALIZER     PTHREAD_ERRORCHECK_MUTEX_INITIALIZER_NP
+#else
+# define SCM_I_PTHREAD_MUTEX_INITIALIZER     PTHREAD_MUTEX_INITIALIZER
+#endif
+
 #define scm_i_pthread_mutex_t               pthread_mutex_t
 #define scm_i_pthread_mutex_init            pthread_mutex_init
 #define scm_i_pthread_mutex_destroy         pthread_mutex_destroy
 #define scm_i_pthread_mutex_trylock         pthread_mutex_trylock
-#define scm_i_pthread_mutex_lock            pthread_mutex_lock
-#define scm_i_pthread_mutex_unlock          pthread_mutex_unlock
+
+#ifdef SCM_USE_ERROR_CHECKING_MUTEXES
+
+# define scm_i_pthread_mutex_lock(m)					\
+   ({									\
+      int mutex_err_;							\
+      mutex_err_ = pthread_mutex_lock (m);				\
+      if (mutex_err_)							\
+	scm_syserror_msg ("mutex_lock", "`pthread_mutex_lock' failed",	\
+			  SCM_EOL, mutex_err_);				\
+      mutex_err_;							\
+    })
+# define scm_i_pthread_mutex_unlock(m)					\
+   ({									\
+      int mutex_err_;							\
+      mutex_err_ = pthread_mutex_unlock (m);				\
+      if (mutex_err_)							\
+	scm_syserror_msg ("mutex_unlock", "`pthread_mutex_unlock' failed", \
+			  SCM_EOL, mutex_err_);				\
+      mutex_err_;							\
+    })
+
+#else /* !SCM_USE_ERROR_CHECKING_MUTEXES */
+
+# define scm_i_pthread_mutex_lock            pthread_mutex_lock
+# define scm_i_pthread_mutex_unlock          pthread_mutex_unlock
+
+#endif /* !SCM_USE_ERROR_CHECKING_MUTEXES */
+
 extern pthread_mutexattr_t scm_i_pthread_mutexattr_recursive[1];
 
 /* Condition variables




[-- Attachment #3: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: thread assert with 1.8.2

[Greg Troxel]

I ran this again (still with 1.8.2) and got a core dump.  There were 4
threads, and #2 was the one that failed.

static scm_t_guile_ticket
scm_leave_guile ()
{
  scm_i_thread *t = suspend ();
  scm_i_pthread_mutex_unlock (&t->heap_mutex);
  return (scm_t_guile_ticket) t;
}
So this is trying to unlock heap_mutex.

There are lots of comments in the  course about how this is trouble.

(gdb) bt
#0  0xbba76c27 in _exit () from /usr/lib/libc.so.12
#1  0xbbaa2fd7 in pthread__errorfunc () from /usr/lib/libpthread.so.0
#2  0xbbaa0ddd in pthread_mutex_unlock () from /usr/lib/libpthread.so.0
#3  0xbbb98760 in scm_leave_guile ()
   from /usr/home/gdt/NetBSD-current/pkgsrc/lang/guile/work/guile-1.8.2/libguile/.libs/libguile.so.17
#4  0xbbb99c52 in on_thread_exit ()
   from /usr/home/gdt/NetBSD-current/pkgsrc/lang/guile/work/guile-1.8.2/libguile/.libs/libguile.so.17
#5  0xbba9fd9c in pthread__destroy_tsd () from /usr/lib/libpthread.so.0
#6  0xbbaa4465 in pthread_exit () from /usr/lib/libpthread.so.0
#7  0xbbaa47a8 in pthread_setcancelstate () from /usr/lib/libpthread.so.0
#8  0xbb9f7700 in swapcontext () from /usr/lib/libc.so.12


with 1.8.1, I get the same thing.

Running threads.test
guile: Error detected by libpthread: Unlocking unlocked mutex.
Detected by file "/usr/home/gdt/NetBSD-current/src/lib/libpthread/pthread_mutex.c", line 357, function "pthread_mutex_unlock".
See pthread(3) for information.
[1]   Abort trap (core dumped) ${dir}${tst}
FAIL: check-guile

(gdb) t 2
[Switching to thread 2 (process 221227)]#0  0xbb9d3e67 in kill ()
   from /usr/lib/libc.so.12
(gdb) bt
#0  0xbb9d3e67 in kill () from /usr/lib/libc.so.12
#1  0xbbaa2fcb in pthread__errorfunc () from /usr/lib/libpthread.so.0
#2  0xbbaa0ddd in pthread_mutex_unlock () from /usr/lib/libpthread.so.0
#3  0xbbb98488 in scm_leave_guile ()
   from /usr/home/gdt/NetBSD-current/pkgsrc/lang/guile/work/guile-1.8.1/libguile/.libs/libguile.so.17
#4  0xbbb9997a in on_thread_exit ()
   from /usr/home/gdt/NetBSD-current/pkgsrc/lang/guile/work/guile-1.8.1/libguile/.libs/libguile.so.17
#5  0xbba9fd9c in pthread__destroy_tsd () from /usr/lib/libpthread.so.0
#6  0xbbaa4465 in pthread_exit () from /usr/lib/libpthread.so.0
#7  0xbbaa47a8 in pthread_setcancelstate () from /usr/lib/libpthread.so.0
#8  0xbb9f7700 in swapcontext () from /usr/lib/libc.so.12

so it seems the same.

1.8.0 does not build easily (some of the pkgsrc patches don't apply, and
it fails without them).




_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: thread assert with 1.8.2

[Greg Troxel]

I read what Kevin wrote about linux threads, and read your patch again,
and I now agree that it should be checking for the same problem that the
NetBSD pthread ipmlementation errored on.

I ran the tests with pthread debugging turned off:

export PTHREAD_DIAGASSERT=Ae
# ignore errors, log to stdout

During the thread test, I got 69 instances of:

guile: Error detected by libpthread: Unlocking unlocked mutex.
Detected by file "/usr/home/gdt/NetBSD-current/src/lib/libpthread/pthread_mutex.c", line 357, function "pthread_mutex_unlock".
See pthread(3) for information.

and the test passed.

Hope this helps.  I do not understand the thread code in guile one
little bit, and short of "grok that and find the bug, either there or in
NetBSD pthread lib", let me know if I can do anything else helpful.



_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: thread assert with 1.8.2

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 2192 bytes --]

Hi,

Greg Troxel <gdt@ir.bbn.com> writes:

> Running threads.test
> guile: Error detected by libpthread: Unlocking unlocked mutex.
> Detected by file "/usr/home/gdt/NetBSD-current/src/lib/libpthread/pthread_mutex.c", line 357, function "pthread_mutex_unlock".
> See pthread(3) for information.
> [1]   Abort trap (core dumped) ${dir}${tst}
>
> The core has several threads, nad this is the one that hit the assert:
>
> (gdb) bt
> #0  0xbb9d6bdb in kill () from /usr/lib/libc.so.12
> #1  0xbbaa3d9b in pthread__errorfunc () from /usr/lib/libpthread.so.0
> #2  0xbbaa1d49 in pthread_mutex_unlock () from /usr/lib/libpthread.so.0
> #3  0xbbb9875c in scm_leave_guile () from /usr/home/gdt/NetBSD-current/pkgsrc/lang/guile/work/guile-1.8.2/libguile/.libs/libguile.so.17
> #4  0xbbb99c4e in on_thread_exit () from /usr/home/gdt/NetBSD-current/pkgsrc/lang/guile/work/guile-1.8.2/libguile/.libs/libguile.so.17
> #5  0xbbaa0e1c in pthread__destroy_tsd () from /usr/lib/libpthread.so.0
> #6  0xbbaa5119 in pthread_exit () from /usr/lib/libpthread.so.0
> #7  0xbbaa5499 in pthread_setcancelstate () from /usr/lib/libpthread.so.0
> #8  0xbb9f9370 in swapcontext () from /usr/lib/libc.so.12

I fixed this one (thank you Greg!) and it turned out to be easier than I
expected: threads were leaving guile mode twice, once in
`scm_i_with_guile_and_parent ()' and then once in `on_thread_exit ()',
hence the above assertion failure.

But now, there's a similar issue in HEAD with `(ice-9 i18n)':

  PASS: locale objects: make-locale (2 args)
  PASS: locale objects: make-locale (2 args, list)
  PASS: locale objects: make-locale (3 args)
  guile: Error detected by libpthread: Unlocking unlocked mutex.
  Detected by file "/n0/gdt/NetBSD-4/src/lib/libpthread/pthread_mutex.c", line 363, function "pthread_mutex_unlock".

Backtrace:

  #0  0xbba68fff in kill () from /usr/lib/libc.so.12
  #1  0xbb9dadb3 in pthread__errorfunc () from /usr/lib/libpthread.so.0
  #2  0xbb9d93f8 in pthread_mutex_unlock () from /usr/lib/libpthread.so.0
  #3  0xbb9cbaa0 in scm_make_locale (category_list=0x2, locale_name=0x80d6870, base_locale=0x204) at ../../libguile/i18n.c:436

Needs further investigation...

Thanks,
Ludovic.


[-- Attachment #2: The patch --]
[-- Type: text/x-patch, Size: 1877 bytes --]

--- orig/ChangeLog
+++ mod/ChangeLog
@@ -1,6 +1,6 @@
 2007-10-02  Ludovic Courtès  <ludo@gnu.org>
 
-	* NEWS: Mention `(ice-9 slib)' fix.
+	* NEWS: Mention `(ice-9 slib)' fix and threading fix.
 
 2007-09-03  Ludovic Courtès  <ludo@gnu.org>
 


--- orig/NEWS
+++ mod/NEWS
@@ -46,6 +46,7 @@
 ** Warnings about duplicate bindings now go to stderr
 ** A memory leak in `make-socket-address' was fixed
 ** Alignment issues (e.g., on SPARC) in network routines were fixed
+** A threading issue that showed up at least on NetBSD was fixed
 ** Build problems on Solaris fixed
 
 * Implementation improvements


--- orig/libguile/ChangeLog
+++ mod/libguile/ChangeLog
@@ -1,3 +1,9 @@
+2007-10-02  Ludovic Courtès  <ludo@gnu.org>
+
+	* threads.c (on_thread_exit): Don't call `scm_leave_guile ()'
+	since we're already in non-guile mode.  Reported by Greg Toxel
+	for NetBSD.
+
 2007-10-01  Ludovic Courtès  <ludo@gnu.org>
 
 	* ports.c (flush_output_port): Expect directly a port instead of


--- orig/libguile/threads.c
+++ mod/libguile/threads.c
@@ -495,20 +495,18 @@
 static void
 on_thread_exit (void *v)
 {
+  /* This handler is executed in non-guile mode.  */
   scm_i_thread *t = (scm_i_thread *)v, **tp;
 
   scm_i_pthread_setspecific (scm_i_thread_key, v);
 
   /* Unblocking the joining threads needs to happen in guile mode
-     since the queue is a SCM data structure.
-  */
+     since the queue is a SCM data structure.  */
   scm_with_guile (do_thread_exit, v);
 
   /* Removing ourself from the list of all threads needs to happen in
      non-guile mode since all SCM values on our stack become
-     unprotected once we are no longer in the list.
-  */
-  scm_leave_guile ();
+     unprotected once we are no longer in the list.  */
   scm_i_pthread_mutex_lock (&thread_admin_mutex);
   for (tp = &all_threads; *tp; tp = &(*tp)->next_thread)
     if (*tp == t)




[-- Attachment #3: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel

Re: thread assert with 1.8.2

[Ludovic Courtès]

[-- Attachment #1: Type: text/plain, Size: 1026 bytes --]

Hi,

ludo@gnu.org (Ludovic Courtès) writes:

> But now, there's a similar issue in HEAD with `(ice-9 i18n)':
>
>   PASS: locale objects: make-locale (2 args)
>   PASS: locale objects: make-locale (2 args, list)
>   PASS: locale objects: make-locale (3 args)
>   guile: Error detected by libpthread: Unlocking unlocked mutex.
>   Detected by file "/n0/gdt/NetBSD-4/src/lib/libpthread/pthread_mutex.c", line 363, function "pthread_mutex_unlock".
>
> Backtrace:
>
>   #0  0xbba68fff in kill () from /usr/lib/libc.so.12
>   #1  0xbb9dadb3 in pthread__errorfunc () from /usr/lib/libpthread.so.0
>   #2  0xbb9d93f8 in pthread_mutex_unlock () from /usr/lib/libpthread.so.0
>   #3  0xbb9cbaa0 in scm_make_locale (category_list=0x2, locale_name=0x80d6870, base_locale=0x204) at ../../libguile/i18n.c:436

OK, got this one too, and also a similar problem in `nl-langinfo' (see
attached patch).

HEAD now seems to work like a charm on NetBSD/i386, though it still
cannot be compiled with `-Werror' (patches welcome!).

Thanks,
Ludovic.


[-- Attachment #2: The patch --]
[-- Type: text/x-patch, Size: 1257 bytes --]

--- orig/libguile/ChangeLog
+++ mod/libguile/ChangeLog
@@ -1,3 +1,13 @@
+2007-10-04  Ludovic Courtès  <ludo@gnu.org>
+
+	* i18n.c (scm_make_locale)[!USE_GNU_LOCALE_API]: Don't call
+	`leave_locale_section ()' on failure of
+	`enter_locale_section ()' since the mutex is not held and locale
+	settings are unchanged.
+	(scm_nl_langinfo)[!USE_GNU_LOCALE_API]: Use
+	`restore_locale_settings ()' instead of `leave_locale_section ()' 
+	since the mutex is not held.
+	
 2007-10-02  Ludovic Courtès  <ludo@gnu.org>
 
 	* threads.c (on_thread_exit): Don't call `scm_leave_guile ()'


--- orig/libguile/i18n.c
+++ mod/libguile/i18n.c
@@ -685,12 +685,14 @@
     scm_t_locale_settings prev_locale;
 
     err = enter_locale_section (c_locale, &prev_locale);
-    leave_locale_section (&prev_locale);
 
     if (err)
       goto fail;
     else
-      SCM_NEWSMOB (locale, scm_tc16_locale_smob_type, c_locale);
+      {
+	leave_locale_section (&prev_locale);
+	SCM_NEWSMOB (locale, scm_tc16_locale_smob_type, c_locale);
+      }
   }
 
 #endif
@@ -1410,7 +1412,7 @@
 	{
 	  c_result = nl_langinfo (c_item);
 
-	  leave_locale_section (&lsec_prev_locale);
+	  restore_locale_settings (&lsec_prev_locale);
 	  free_locale_settings (&lsec_prev_locale);
 	}
 #endif




[-- Attachment #3: Type: text/plain, Size: 143 bytes --]

_______________________________________________
Guile-devel mailing list
Guile-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/guile-devel