[Patch] --with-threads on MinGW

[Patch] --with-threads on MinGW

[Nils Durner]

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

Hi,

the attached patch fixes building on MinGW with "--with-threads=pthreads"


    Nils Durner

[-- Attachment #2: guile_mingw01.diff --]
[-- Type: text/plain, Size: 2650 bytes --]

diff -Naur guile-1.8.1/libguile/gen-scmconfig.c guile-1.8.1.nd/libguile/gen-scmconfig.c
--- guile-1.8.1/libguile/gen-scmconfig.c	Sun Aug 27 21:05:24 2006
+++ guile-1.8.1.nd/libguile/gen-scmconfig.c	Sun Dec  3 14:50:05 2006
@@ -355,7 +355,7 @@
 
   pf ("\n");
   pf ("/* same as POSIX \"struct timespec\" -- always defined */\n");
-#ifdef HAVE_STRUCT_TIMESPEC
+#if HAVE_STRUCT_TIMESPEC || SCM_I_GSC_USE_PTHREAD_THREADS
   pf ("typedef struct timespec scm_t_timespec;\n");
 #else
   pf ("/* POSIX.4 structure for a time value.  This is like a `struct timeval'"
diff -Naur guile-1.8.1/libguile/scmsigs.c guile-1.8.1.nd/libguile/scmsigs.c
--- guile-1.8.1/libguile/scmsigs.c	Sat Feb 18 06:08:46 2006
+++ guile-1.8.1.nd/libguile/scmsigs.c	Sun Dec  3 12:52:43 2006
@@ -46,11 +46,14 @@
 
 #ifdef __MINGW32__
 #include <windows.h>
+#include <fcntl.h>
+#include <process.h>
 #define alarm(sec) (0)
 /* This weird comma expression is because Sleep is void under Windows. */
 #define sleep(sec) (Sleep ((sec) * 1000), 0)
 #define usleep(usec) (Sleep ((usec) / 1000), 0)
 #define kill(pid, sig) raise (sig)
+#define pipe(fd) _pipe (fd, 256, O_BINARY)
 #endif
 
 \f
@@ -149,12 +152,15 @@
 static SCM
 signal_delivery_thread (void *data)
 {
-  sigset_t all_sigs;
   int n, sig;
   char sigbyte;
 
+#ifndef __MINGW32__
+  sigset_t all_sigs;
+
   sigfillset (&all_sigs);
   scm_i_pthread_sigmask (SIG_SETMASK, &all_sigs, NULL);
+#endif
 
   while (1)
     {
diff -Naur guile-1.8.1/libguile/threads.c guile-1.8.1.nd/libguile/threads.c
--- guile-1.8.1/libguile/threads.c	Sat Jul 29 18:00:17 2006
+++ guile-1.8.1.nd/libguile/threads.c	Sun Dec  3 15:42:05 2006
@@ -143,7 +143,13 @@
 {
   scm_i_thread *t = SCM_I_THREAD_DATA (exp);
   scm_puts ("#<thread ", port);
-  scm_uintprint ((size_t)t->pthread, 10, port);
+  scm_uintprint ((size_t) 
+#ifndef __MINGW32__
+	t->pthread,
+#else
+	t->pthread.p,
+#endif
+	10, port);
   scm_puts (" (", port);
   scm_uintprint ((scm_t_bits)t, 16, port);
   scm_puts (")>", port);
@@ -572,13 +578,16 @@
 
 #if SCM_USE_PTHREAD_THREADS
 /* pthread_getattr_np not available on MacOS X and Solaris 10. */
-#if HAVE_PTHREAD_ATTR_GETSTACK && HAVE_PTHREAD_GETATTR_NP
+#if (HAVE_PTHREAD_ATTR_GETSTACK && HAVE_PTHREAD_GETATTR_NP) || __MINGW32__
 
 #define HAVE_GET_THREAD_STACK_BASE
 
 static SCM_STACKITEM *
 get_thread_stack_base ()
 {
+#ifdef __MINGW32__
+    return scm_get_stack_base ();
+#else
   pthread_attr_t attr;
   void *start, *end;
   size_t size;
@@ -604,8 +613,8 @@
       return end;
 #endif
     }
+#endif // MINGW
 }
-
 #endif /* HAVE_PTHREAD_ATTR_GETSTACK && HAVE_PTHREAD_GETATTR_NP */
 
 #else /* !SCM_USE_PTHREAD_THREADS */

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

_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Kevin Ryde]

Nils Durner <ndurner@web.de> writes:
>
> the attached patch fixes building on MinGW with "--with-threads=pthreads"

Looks good in principle, but some bits ought to be in a more autoconfy
style.

> -#ifdef HAVE_STRUCT_TIMESPEC
> +#if HAVE_STRUCT_TIMESPEC || SCM_I_GSC_USE_PTHREAD_THREADS
>    pf ("typedef struct timespec scm_t_timespec;\n");

Does the detection of struct timespec go wrong somehow?

> +#ifndef __MINGW32__
> +  sigset_t all_sigs;
> +
>    sigfillset (&all_sigs);
>    scm_i_pthread_sigmask (SIG_SETMASK, &all_sigs, NULL);
> +#endif

I think I'd rather either conditionalize on the existance of
pthread_sigmask, or perhaps make some dummy sigset stuff if it doesn't
exist.

> +#ifndef __MINGW32__
> +	t->pthread,
> +#else
> +	t->pthread.p,
> +#endif

What does that do?

>  #if SCM_USE_PTHREAD_THREADS
>  /* pthread_getattr_np not available on MacOS X and Solaris 10. */
> -#if HAVE_PTHREAD_ATTR_GETSTACK && HAVE_PTHREAD_GETATTR_NP
> +#if (HAVE_PTHREAD_ATTR_GETSTACK && HAVE_PTHREAD_GETATTR_NP) || __MINGW32__

Remind us what's wrong with the getstack detection ...


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Nils Durner]

>> -#ifdef HAVE_STRUCT_TIMESPEC
>> +#if HAVE_STRUCT_TIMESPEC || SCM_I_GSC_USE_PTHREAD_THREADS
>> pf ("typedef struct timespec scm_t_timespec;\n");
>
> Does the detection of struct timespec go wrong somehow?
The only timespec declaration on Win32 is the one in pthread.h, that's
why I or'ed  SCM_I_GSC_USE_PTHREAD_THREADS.
scm's own timespec declaration is considered "incompatible" for some reason.

>> +#ifndef __MINGW32__
>> + sigset_t all_sigs;
>> +
>> sigfillset (&all_sigs);
>> scm_i_pthread_sigmask (SIG_SETMASK, &all_sigs, NULL);
>> +#endif
>
> I think I'd rather either conditionalize on the existance of
> pthread_sigmask, or perhaps make some dummy sigset stuff if it doesn't
> exist.
OK, makes sense.

>> +#ifndef __MINGW32__
>> + t->pthread,
>> +#else
>> + t->pthread.p,
>> +#endif
>
> What does that do?
Pthreads-win32 defines pthread_t as struct.

>> #if SCM_USE_PTHREAD_THREADS
>> /* pthread_getattr_np not available on MacOS X and Solaris 10. */
>> -#if HAVE_PTHREAD_ATTR_GETSTACK && HAVE_PTHREAD_GETATTR_NP
>> +#if (HAVE_PTHREAD_ATTR_GETSTACK && HAVE_PTHREAD_GETATTR_NP) ||
>> __MINGW32__
>
> Remind us what's wrong with the getstack detection ...
pthread_attr_getstack() exists, but pthread_getattr_np() doesn't.
This is just to get the #define HAVE_GET_... and the definition of
get_thread_stack_base().


Nils



_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Kevin Ryde]

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

Nils Durner <ndurner@web.de> writes:
>
> The only timespec declaration on Win32 is the one in pthread.h,

(Taking bits one at a time so I don't get confused.)

I checked in the change below to hopefully detect.  My debian mingw
cross doesn't have pthread.h at all, so I can't actually test it.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: configure.in.pthread-timespec.diff --]
[-- Type: text/x-diff, Size: 1559 bytes --]

--- configure.in.~1.268.2.23.~	2006-12-03 10:19:27.000000000 +1100
+++ configure.in	2006-12-14 10:10:18.000000000 +1100
@@ -622,10 +622,12 @@
 # Reasons for testing:
 #   netdb.h - not in mingw
 #   sys/param.h - not in mingw
+#   pthread.h - only available with pthreads.  ACX_PTHREAD doesn't
+#       check this specifically, we need it for the timespec test below.
 #   sethostname - the function itself check because it's not in mingw,
 #       the DECL is checked because Solaris 10 doens't have in any header
 #
-AC_CHECK_HEADERS(crypt.h netdb.h sys/param.h sys/resource.h sys/file.h)
+AC_CHECK_HEADERS(crypt.h netdb.h pthread.h sys/param.h sys/resource.h sys/file.h)
 AC_CHECK_FUNCS(chroot flock getlogin cuserid getpriority setpriority getpass sethostname gethostname)
 AC_CHECK_DECLS([sethostname])
 
@@ -1034,17 +1036,22 @@
 fi
 
 
+# On mingw, struct timespec is in <pthread.h>.
+#
 AC_MSG_CHECKING(for struct timespec)
 AC_CACHE_VAL(scm_cv_struct_timespec,
 	AC_TRY_COMPILE([
-#include <time.h>],
+#include <time.h>
+#if HAVE_PTHREAD_H
+#include <pthread.h>
+#endif],
 			[struct timespec t;  t.tv_nsec = 100],
 			scm_cv_struct_timespec="yes",
 			scm_cv_struct_timespec="no"))
 AC_MSG_RESULT($scm_cv_struct_timespec)
 if test $scm_cv_struct_timespec = yes; then
   AC_DEFINE(HAVE_STRUCT_TIMESPEC, 1,
-    [Define this if your system defines struct timespec via <time.h>.])
+    [Define this if your system defines struct timespec via either <time.h> or <pthread.h>.])
 fi
 
 #--------------------------------------------------------------------

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

_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Kevin Ryde]

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

Nils Durner <ndurner@web.de> writes:
>
> Pthreads-win32 defines pthread_t as struct.

I made the change below to go via a union to pick out some info.  Does
it look about right?  Maybe there's an easier way to pick out the
bytes ...


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: configure.in.pthread-timespec.diff --]
[-- Type: text/x-diff, Size: 1559 bytes --]

--- configure.in.~1.268.2.23.~	2006-12-03 10:19:27.000000000 +1100
+++ configure.in	2006-12-14 10:10:18.000000000 +1100
@@ -622,10 +622,12 @@
 # Reasons for testing:
 #   netdb.h - not in mingw
 #   sys/param.h - not in mingw
+#   pthread.h - only available with pthreads.  ACX_PTHREAD doesn't
+#       check this specifically, we need it for the timespec test below.
 #   sethostname - the function itself check because it's not in mingw,
 #       the DECL is checked because Solaris 10 doens't have in any header
 #
-AC_CHECK_HEADERS(crypt.h netdb.h sys/param.h sys/resource.h sys/file.h)
+AC_CHECK_HEADERS(crypt.h netdb.h pthread.h sys/param.h sys/resource.h sys/file.h)
 AC_CHECK_FUNCS(chroot flock getlogin cuserid getpriority setpriority getpass sethostname gethostname)
 AC_CHECK_DECLS([sethostname])
 
@@ -1034,17 +1036,22 @@
 fi
 
 
+# On mingw, struct timespec is in <pthread.h>.
+#
 AC_MSG_CHECKING(for struct timespec)
 AC_CACHE_VAL(scm_cv_struct_timespec,
 	AC_TRY_COMPILE([
-#include <time.h>],
+#include <time.h>
+#if HAVE_PTHREAD_H
+#include <pthread.h>
+#endif],
 			[struct timespec t;  t.tv_nsec = 100],
 			scm_cv_struct_timespec="yes",
 			scm_cv_struct_timespec="no"))
 AC_MSG_RESULT($scm_cv_struct_timespec)
 if test $scm_cv_struct_timespec = yes; then
   AC_DEFINE(HAVE_STRUCT_TIMESPEC, 1,
-    [Define this if your system defines struct timespec via <time.h>.])
+    [Define this if your system defines struct timespec via either <time.h> or <pthread.h>.])
 fi
 
 #--------------------------------------------------------------------

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

_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Kevin Ryde]

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

I wrote:
>
> I made the change below

Oops, too much cut and paste.  Actual change is:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: threads.c.print-struct.diff --]
[-- Type: text/x-diff, Size: 1375 bytes --]

Index: threads.c
===================================================================
RCS file: /cvsroot/guile/guile/guile-core/libguile/threads.c,v
retrieving revision 1.84.2.5
retrieving revision 1.84.2.6
diff -u -r1.84.2.5 -r1.84.2.6
--- threads.c	25 Jul 2006 00:09:30 -0000	1.84.2.5
+++ threads.c	13 Dec 2006 23:55:51 -0000	1.84.2.6
@@ -141,9 +141,32 @@
 static int
 thread_print (SCM exp, SCM port, scm_print_state *pstate SCM_UNUSED)
 {
+  /* On a Gnu system pthread_t is an unsigned long, but on mingw it's a
+     struct.  A cast like "(unsigned long) t->pthread" is a syntax error in
+     the struct case, hence we go via a union, and extract according to the
+     size of pthread_t.  */
+  union {
+    pthread_t p;
+    unsigned short us;
+    unsigned int   ui;
+    unsigned long  ul;
+    scm_t_uintmax  um;
+  } u;
   scm_i_thread *t = SCM_I_THREAD_DATA (exp);
+  scm_i_pthread_t p = t->pthread;
+  scm_t_uintmax id;
+  u.p = p;
+  if (sizeof (p) == sizeof (unsigned short))
+    id = u.us;
+  else if (sizeof (p) == sizeof (unsigned int))
+    id = u.ui;
+  else if (sizeof (p) == sizeof (unsigned long))
+    id = u.ul;
+  else
+    id = u.um;
+
   scm_puts ("#<thread ", port);
-  scm_uintprint ((size_t)t->pthread, 10, port);
+  scm_uintprint (id, 10, port);
   scm_puts (" (", port);
   scm_uintprint ((scm_t_bits)t, 16, port);
   scm_puts (")>", port);

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

_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Nils Durner]

> I made the change below to go via a union to pick out some info.  Does
> it look about right?

Yes, looks good. However, pthread.h has to be included somewhere to have
timespec declared.
Otherwise, building fails with

    threads.c: In function `scm_timed_wait_condition_variable':
    threads.c:1264: error: storage size of 'waittime' isn't known


Best,

Nils Durner


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Kevin Ryde]

Nils Durner <ndurner@web.de> writes:
>
> However, pthread.h has to be included somewhere to have
> timespec declared.
> Otherwise, building fails with
>
>     threads.c: In function `scm_timed_wait_condition_variable':
>     threads.c:1264: error: storage size of 'waittime' isn't known

It should come from pthread-threads.h, which is included by threads.h
for SCM_USE_PTHREAD_THREADS.  Can you check if that happens.


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Kevin Ryde]

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

Nils Durner <ndurner@web.de> writes:
>
> pthread_attr_getstack() exists, but pthread_getattr_np() doesn't.

How about the change below (untested), to keep out of the hair of the
_np bits.

That getattr_np is confusing.  Is the right way to create an attr
object with pthread_attr_init, use it for pthread_create, then use it
again later with pthread_attr_getstack if you want to ask something?
If that could be passed down through the levels of guile inits it'd
get rid of a "non-portable" call.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: threads.c.mingw-stack-base.diff --]
[-- Type: text/x-diff, Size: 538 bytes --]

--- threads.c.~1.84.2.6.~	2006-12-14 10:52:15.000000000 +1100
+++ threads.c	2006-12-14 11:36:37.000000000 +1100
@@ -629,7 +629,16 @@
     }
 }
 
-#endif /* HAVE_PTHREAD_ATTR_GETSTACK && HAVE_PTHREAD_GETATTR_NP */
+#elif defined (__MINGW32__)
+/* In mingw the basic scm_get_stack_base can be used in any thread. */
+#define HAVE_GET_THREAD_STACK_BASE
+static SCM_STACKITEM *
+get_thread_stack_base ()
+{
+  return scm_get_stack_base ();
+}
+
+#endif /* pthread methods of get_thread_stack_base */
 
 #else /* !SCM_USE_PTHREAD_THREADS */
 

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

_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Kevin Ryde]

Nils Durner <ndurner@web.de> writes:
>
>  static SCM
>  signal_delivery_thread (void *data)
>  {
> -  sigset_t all_sigs;
>    int n, sig;
>    char sigbyte;
>  
> +#ifndef __MINGW32__
> +  sigset_t all_sigs;
> +
>    sigfillset (&all_sigs);
>    scm_i_pthread_sigmask (SIG_SETMASK, &all_sigs, NULL);
> +#endif

Are you sure about that?  Does it mean the signal delivery thread
doesn't catch any signals?


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Nils Durner]

> Are you sure about that? Does it mean the signal delivery thread
> doesn't catch any signals?
Yes, as said before, the concept of signals is unknown under Windows.


Best,

Nils Durner


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Kevin Ryde]

Nils Durner <ndurner@web.de> writes:
>
> Yes, as said before, the concept of signals is unknown under Windows.

Oops, ok, there's a local notion through raise(), but nothing that you
want a separate running thread for.


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile

Re: [Patch] --with-threads on MinGW

[Kevin Ryde]

I made the sigmask change.  That should be all of what you reported,
thanks.

But you could give it a run when you get a chance, I'm not sure if
that signal delivery thread will do the right thing.  I'm guessing
it'll be harmless but useless.  It might be better to go with the null
threads code.  The test suite doesn't really exercise signals, you
might have to make your own program with some raises or alarms.


_______________________________________________
Bug-guile mailing list
Bug-guile@gnu.org
http://lists.gnu.org/mailman/listinfo/bug-guile