public inbox for cygwin@cygwin.com
 help / color / mirror / Atom feed
* Re: [ANNOUNCEMENT] python3 3.6.1-2 (x86 only)
       [not found] <20170324182610.8B2F9440070@conbox-039.nifty.com>
@ 2017-03-25  6:10 ` Takashi Yano
  2017-03-28 12:38   ` Takashi Yano
  0 siblings, 1 reply; 3+ messages in thread
From: Takashi Yano @ 2017-03-25  6:10 UTC (permalink / raw)
  To: cygwin

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

Hello,

On Fri, 24 Mar 2017 13:22:56 -0500 Yaakov Selkowitz wrote:
> This release fixes thread allocation issues reported with WoW64 on some 
> versions of Windows.  As this only affects 32-bit Cygwin, the 64-bit build
> has not been updated.

Thanks for quick fix. I have confirmed the issue has been fixed.

However, I have found another prombem related this issue.

Python API PyThread_create_key() in the latest release of 64-bit version
returns an invalid key value, which is truncated into int size.

To confirm this, execute following script:
--- from here ---
from ctypes import pythonapi
key = pythonapi.PyThread_create_key()
res1 = pythonapi.PyThread_set_key_value(key, 5555)
res2 = pythonapi.PyThread_get_key_value(key)
res3 = pythonapi.PyThread_delete_key(key)
print(key,res1,res2,res3)
--- to here ---

Expected result is:
1 0 5555 0

'1' in the first column can be any other value, but the remaining columns
must be '0 5555 0'.

However, result of 64-bit version is:
1145600 -1 0 22

This means PyThread_set_key_value(), PyThread_get_key_value() and
PyThread_delete_key() fail to access key 1145600. This is because the key value is invalid. It seems that the 64 bit key value is truncated into
32-bit value.

To fix this issue as well as the issue on WOW, I have made two patches
attached, based on two different ideas.

pthread-cygwin-1.patch:
The higher part of key value is saved, and only lower part is handed
to python API. The key value is recombined with higher part when it
handed to cygwin pthread functions.
Smaller memory and little bit faster.

pthread-cygwin-2.patch:
The key value returned by cygwin pthread_key_create() is saved in
table, and index of the table is handed to python API as a key.
The key value is converted via the table when it handed to cygwin
pthread functions.
Larger memory but steady operation.

I prefer pthread-cygwin-2.patch. What do you think?

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: pthread-cygwin-1.patch --]
[-- Type: application/octet-stream, Size: 2366 bytes --]

--- origsrc/Python-3.6.1/Python/thread_pthread.h	2017-03-25 08:10:30.784601800 +0900
+++ src/Python-3.6.1/Python/thread_pthread.h	2017-03-25 08:43:12.070906900 +0900
@@ -603,6 +603,67 @@
 
 #define Py_HAVE_NATIVE_TLS
 
+#ifdef __CYGWIN__
+/* Cygwin pthread TLS key type is a pointer, whereas Python 3 assumes
+ * int type. So wrapper functions are used instead.
+ */
+static unsigned long keybase = ULONG_MAX;
+
+static int
+py_pthread_key_create(unsigned long *key, void(*func)(void *))
+{
+    int ret;
+    const unsigned long mask = ULONG_MAX & ~(unsigned long)INT_MAX;
+    unsigned long keybase_new;
+    pthread_key_t key_cyg;
+    ret = pthread_key_create(&key_cyg, func);
+    if (ret) {
+        /* Error */
+        *key = 0;
+        return ret;
+    }
+    keybase_new = (unsigned long)key_cyg & mask;
+    /* If base address is different, treat as error */
+    if (keybase != ULONG_MAX && keybase_new != keybase) {
+        pthread_key_delete(key_cyg);
+        *key = 0;
+        errno = ENOMEM;
+        return ENOMEM;
+    }
+    keybase = keybase_new;
+    *key = (unsigned long)key_cyg & ~mask;
+    return 0;
+}
+
+static int
+py_pthread_key_delete(unsigned long key)
+{
+    pthread_key_t key_cyg = (pthread_key_t)(keybase | key);
+    return pthread_key_delete(key_cyg);
+}
+
+static int
+py_pthread_setspecific(unsigned long key, const void *p)
+{
+    pthread_key_t key_cyg = (pthread_key_t)(keybase | key);
+    return pthread_setspecific(key_cyg, p);
+}
+
+static void *
+py_pthread_getspecific(unsigned long key)
+{
+    pthread_key_t key_cyg = (pthread_key_t)(keybase | key);
+    return pthread_getspecific(key_cyg);
+}
+
+#define pthread_key_t unsigned long
+#define pthread_key_create py_pthread_key_create
+#define pthread_key_delete py_pthread_key_delete
+#define pthread_setspecific py_pthread_setspecific
+#define pthread_getspecific py_pthread_getspecific
+
+#endif /* __CYGWIN__ */
+
 long
 PyThread_create_key(void)
 {
@@ -610,15 +671,12 @@
     int fail = pthread_key_create(&key, NULL);
     if (fail)
         return -1L;
-#ifndef __CYGWIN__
-    /* Cygwin pthread types are pointers, which may "overflow" signed long */
     if (key > LONG_MAX) {
         /* Issue #22206: handle integer overflow */
         pthread_key_delete(key);
         errno = ENOMEM;
         return -1L;
     }
-#endif
     return (long)key;
 }
 

[-- Attachment #3: pthread-cygwin-2.patch --]
[-- Type: application/octet-stream, Size: 2483 bytes --]

--- origsrc/Python-3.6.1/Python/thread_pthread.h	2017-03-25 08:10:30.784601800 +0900
+++ src/Python-3.6.1/Python/thread_pthread.h	2017-03-25 08:44:21.946596600 +0900
@@ -603,6 +603,80 @@
 
 #define Py_HAVE_NATIVE_TLS
 
+#ifdef __CYGWIN__
+/* Cygwin pthread TLS key type is a pointer, whereas Python 3 assumes
+ * int type. So wrapper functions are used instead.
+ */
+static pthread_key_t key_tbl[PTHREAD_KEYS_MAX];
+
+static int
+py_pthread_key_create(unsigned long *key, void(*func)(void *))
+{
+    int ret;
+    int i;
+    pthread_key_t key_cyg;
+    ret = pthread_key_create(&key_cyg, func);
+    if (ret) {
+        /* Error */
+        *key = 0;
+        return ret;
+    }
+    for (i=0; i<PTHREAD_KEYS_MAX; i++) {
+        if (key_tbl[i] == NULL) {
+            /* Succeeded */
+            *key = i;
+            key_tbl[i] = key_cyg;
+            return 0;
+        }
+    }
+    pthread_key_delete(key_cyg);
+    /* PTHREAD_KEYS_MAX reached */
+    *key = 0;
+    errno = EAGAIN;
+    return EAGAIN;
+}
+
+static int
+py_pthread_key_delete(unsigned long key)
+{
+    int ret;
+    if (key >= PTHREAD_KEYS_MAX || key_tbl[key] == NULL) {
+        errno = EINVAL;
+        return EINVAL;
+    }
+    ret = pthread_key_delete(key_tbl[key]);
+    key_tbl[key] = NULL;
+    return ret;
+}
+
+static int
+py_pthread_setspecific(unsigned long key, const void *p)
+{
+    if (key >= PTHREAD_KEYS_MAX || key_tbl[key] == NULL) {
+        errno = EINVAL;
+        return EINVAL;
+    }
+    return pthread_setspecific(key_tbl[key], p);
+}
+
+static void *
+py_pthread_getspecific(unsigned long key)
+{
+    if (key >= PTHREAD_KEYS_MAX || key_tbl[key] == NULL) {
+        errno = EINVAL;
+        return NULL;
+    }
+    return pthread_getspecific(key_tbl[key]);
+}
+
+#define pthread_key_t unsigned long
+#define pthread_key_create py_pthread_key_create
+#define pthread_key_delete py_pthread_key_delete
+#define pthread_setspecific py_pthread_setspecific
+#define pthread_getspecific py_pthread_getspecific
+
+#endif /* __CYGWIN__ */
+
 long
 PyThread_create_key(void)
 {
@@ -610,15 +684,12 @@
     int fail = pthread_key_create(&key, NULL);
     if (fail)
         return -1L;
-#ifndef __CYGWIN__
-    /* Cygwin pthread types are pointers, which may "overflow" signed long */
     if (key > LONG_MAX) {
         /* Issue #22206: handle integer overflow */
         pthread_key_delete(key);
         errno = ENOMEM;
         return -1L;
     }
-#endif
     return (long)key;
 }
 

[-- Attachment #4: Type: text/plain, Size: 219 bytes --]


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [ANNOUNCEMENT] python3 3.6.1-2 (x86 only)
  2017-03-25  6:10 ` [ANNOUNCEMENT] python3 3.6.1-2 (x86 only) Takashi Yano
@ 2017-03-28 12:38   ` Takashi Yano
  0 siblings, 0 replies; 3+ messages in thread
From: Takashi Yano @ 2017-03-28 12:38 UTC (permalink / raw)
  To: cygwin

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

On Sat, 25 Mar 2017 09:46:38 +0900 Takashi Yano wrote:
> from ctypes import pythonapi
> key = pythonapi.PyThread_create_key()
> res1 = pythonapi.PyThread_set_key_value(key, 5555)
> res2 = pythonapi.PyThread_get_key_value(key)
> res3 = pythonapi.PyThread_delete_key(key)
> print(key,res1,res2,res3)

There was one mistake. The function type of PyThread_delete_key() is
void, so the return value is meaningless (just a garbage).

Moreover, the script works as expected if modified as follows.

--- from here ---
from ctypes import *
pythonapi.PyThread_create_key.restype = c_long
pythonapi.PyThread_set_key_value.argtypes = [c_long, c_void_p]
pythonapi.PyThread_get_key_value.restype = c_void_p
pythonapi.PyThread_get_key_value.argtypes = [c_long]
key = pythonapi.PyThread_create_key()
res1 = pythonapi.PyThread_set_key_value(key, 5555)
res2 = pythonapi.PyThread_get_key_value(key)
print(key,res1,res2)
--- to here ---

This is because 3.6-thread-cygwin64.patch is changing the type of
functions as the key type is changed from int to long.

However, from the portability point of view, it is not desirable to
change the types of functions exported as APIs.

If one of the idea of the patches I proposed will be employed, the
patch 3.6-thread-cygwin64.patch is not necessary anymore.

Thus, I would like to propose new version of the patches replacing
3.6-thread-cygwin64.patch.

3.6-thread-cygwin-replacement-1.patch:
  replaces 3.6-thread-cygwin64.patch and pthread-cygwin-1.patch

3.6-thread-cygwin-replacement-2.patch:
  replaces 3.6-thread-cygwin64.patch and pthread-cygwin-2.patch

I would appreciate your comment.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

[-- Attachment #2: 3.6-thread-cygwin-replacement-1.patch --]
[-- Type: application/octet-stream, Size: 1906 bytes --]

--- origsrc/Python-3.6.1/Python/thread_pthread.h	2017-03-25 08:10:30.784601800 +0900
+++ src/Python-3.6.1/Python/thread_pthread.h	2017-03-25 08:43:12.070906900 +0900
@@ -603,6 +603,64 @@
 
 #define Py_HAVE_NATIVE_TLS
 
+#ifdef __CYGWIN__
+/* Cygwin pthread TLS key type is a pointer, whereas Python 3 assumes
+ * int type. So wrapper functions are used instead.
+ */
+static unsigned long keybase = ULONG_MAX;
+
+static int
+py_pthread_key_create(unsigned int *key, void(*func)(void *))
+{
+    int ret;
+    const unsigned long mask = ULONG_MAX & ~(unsigned long)INT_MAX;
+    unsigned long keybase_new;
+    pthread_key_t key_cyg;
+    ret = pthread_key_create(&key_cyg, func);
+    if (ret) {
+        /* Error */
+        return ret;
+    }
+    keybase_new = (unsigned long)key_cyg & mask;
+    /* If base address is different, treat as error */
+    if (keybase != ULONG_MAX && keybase_new != keybase) {
+        pthread_key_delete(key_cyg);
+        return ENOMEM;
+    }
+    keybase = keybase_new;
+    *key = (unsigned int)((unsigned long)key_cyg & ~mask);
+    return 0;
+}
+
+static int
+py_pthread_key_delete(unsigned int key)
+{
+    pthread_key_t key_cyg = (pthread_key_t)(keybase | key);
+    return pthread_key_delete(key_cyg);
+}
+
+static int
+py_pthread_setspecific(unsigned int key, const void *p)
+{
+    pthread_key_t key_cyg = (pthread_key_t)(keybase | key);
+    return pthread_setspecific(key_cyg, p);
+}
+
+static void *
+py_pthread_getspecific(unsigned int key)
+{
+    pthread_key_t key_cyg = (pthread_key_t)(keybase | key);
+    return pthread_getspecific(key_cyg);
+}
+
+#define pthread_key_t unsigned int
+#define pthread_key_create py_pthread_key_create
+#define pthread_key_delete py_pthread_key_delete
+#define pthread_setspecific py_pthread_setspecific
+#define pthread_getspecific py_pthread_getspecific
+
+#endif /* __CYGWIN__ */
+
 int
 PyThread_create_key(void)
 {


[-- Attachment #3: 3.6-thread-cygwin-replacement-2.patch --]
[-- Type: application/octet-stream, Size: 1940 bytes --]

--- origsrc/Python-3.6.1/Python/thread_pthread.h	2017-03-25 08:10:30.784601800 +0900
+++ src/Python-3.6.1/Python/thread_pthread.h	2017-03-25 08:44:21.946596600 +0900
@@ -603,6 +603,74 @@
 
 #define Py_HAVE_NATIVE_TLS
 
+#ifdef __CYGWIN__
+/* Cygwin pthread TLS key type is a pointer, whereas Python 3 assumes
+ * int type. So wrapper functions are used instead.
+ */
+static pthread_key_t key_tbl[PTHREAD_KEYS_MAX];
+
+static int
+py_pthread_key_create(unsigned int *key, void(*func)(void *))
+{
+    int ret;
+    int i;
+    pthread_key_t key_cyg;
+    ret = pthread_key_create(&key_cyg, func);
+    if (ret) {
+        /* Error */
+        return ret;
+    }
+    for (i=0; i<PTHREAD_KEYS_MAX; i++) {
+        if (key_tbl[i] == NULL) {
+            /* Succeeded */
+            key_tbl[i] = key_cyg;
+            *key = i;
+            return 0;
+        }
+    }
+    pthread_key_delete(key_cyg);
+    /* PTHREAD_KEYS_MAX reached */
+    return EAGAIN;
+}
+
+static int
+py_pthread_key_delete(unsigned int key)
+{
+    int ret;
+    if (key >= PTHREAD_KEYS_MAX || key_tbl[key] == NULL) {
+        return EINVAL;
+    }
+    ret = pthread_key_delete(key_tbl[key]);
+    key_tbl[key] = NULL;
+    return ret;
+}
+
+static int
+py_pthread_setspecific(unsigned int key, const void *p)
+{
+    if (key >= PTHREAD_KEYS_MAX || key_tbl[key] == NULL) {
+        return EINVAL;
+    }
+    return pthread_setspecific(key_tbl[key], p);
+}
+
+static void *
+py_pthread_getspecific(unsigned int key)
+{
+    if (key >= PTHREAD_KEYS_MAX || key_tbl[key] == NULL) {
+        return NULL;
+    }
+    return pthread_getspecific(key_tbl[key]);
+}
+
+#define pthread_key_t unsigned int
+#define pthread_key_create py_pthread_key_create
+#define pthread_key_delete py_pthread_key_delete
+#define pthread_setspecific py_pthread_setspecific
+#define pthread_getspecific py_pthread_getspecific
+
+#endif /* __CYGWIN__ */
+
 int
 PyThread_create_key(void)
 {


[-- Attachment #4: Type: text/plain, Size: 219 bytes --]


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [ANNOUNCEMENT] python3 3.6.1-2 (x86 only)
@ 2017-03-24 19:58 Yaakov Selkowitz
  0 siblings, 0 replies; 3+ messages in thread
From: Yaakov Selkowitz @ 2017-03-24 19:58 UTC (permalink / raw)
  To: cygwin

The following packages have been uploaded to the Cygwin x86 distribution:

* python3-3.6.1-2
* python3-devel-3.6.1-2
* python3-test-3.6.1-2
* python3-tkinter-3.6.1-2
* idle3-3.6.1-2

Python is an interpreted, interactive, object-oriented programming 
language. It incorporates modules, exceptions, dynamic typing, very high 
level dynamic data types, and classes. Python combines remarkable power 
with very clear syntax. It has interfaces to many system calls and 
libraries, as well as to various window systems, and is extensible in C or 
C++. It is also usable as an extension language for applications that need 
a programmable interface.

This release fixes thread allocation issues reported with WoW64 on some 
versions of Windows.  As this only affects 32-bit Cygwin, the 64-bit build
has not been updated.

--
Yaakov

--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-03-28 11:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20170324182610.8B2F9440070@conbox-039.nifty.com>
2017-03-25  6:10 ` [ANNOUNCEMENT] python3 3.6.1-2 (x86 only) Takashi Yano
2017-03-28 12:38   ` Takashi Yano
2017-03-24 19:58 Yaakov Selkowitz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).