Fix open/fopen in thread other than main

Before this patch, calling Volume.{f}open in a thread other than main
thread was causing segmentation fault (test included).  The reason is
missing ctypes declarations.

Also, this patch fixes errno handling for these two functions, making
couple of FIXME/TODO notes go away.

Change-Id: Iae9638b7d16cc0e0c587fd21a94be677f2d4af59
Signed-off-by: Oleksiy Syvokon <oleksiy.syvokon@gmail.com>

4 files changed, 33 insertions, 20 deletions

diff --git a/gluster/api.py b/gluster/api.py
index 62135c5..c286945 100755
--- a/gluster/api.py
+++ b/gluster/api.py
@@ -271,6 +271,18 @@ glfs_set_logging = ctypes.CFUNCTYPE(ctypes.c_int,
 glfs_fini = ctypes.CFUNCTYPE(
     ctypes.c_int, ctypes.c_void_p)(('glfs_fini', client))
 
+glfs_creat = ctypes.CFUNCTYPE(ctypes.c_void_p,
+                              ctypes.c_void_p,
+                              ctypes.c_char_p,
+                              ctypes.c_int,
+                              ctypes.c_uint,
+                              use_errno=True)(('glfs_creat', client))
+
+glfs_open = ctypes.CFUNCTYPE(ctypes.c_void_p,
+                             ctypes.c_void_p,
+                             ctypes.c_char_p,
+                             ctypes.c_int,
+                             use_errno=True)(('glfs_open', client))
 
 glfs_close = ctypes.CFUNCTYPE(
     ctypes.c_int, ctypes.c_void_p)(('glfs_close', client))
@@ -444,12 +456,6 @@ glfs_getcwd = ctypes.CFUNCTYPE(ctypes.c_char_p,
                                ctypes.c_size_t)(('glfs_getcwd', client))
 
 
-# TODO: creat and open fails on test_create_file_already_exists & test_open_file_not_exist functional testing, # noqa
-# when defined via function prototype.. Need to find RCA. For time being, using it from 'api.glfs_' # noqa
-#_glfs_creat = ctypes.CFUNCTYPE(ctypes.c_void_p, ctypes.c_void_p, ctypes.c_char_p, ctypes.c_int, ctypes.c_uint) # noqa
-                              # (('glfs_creat', client)) # noqa
-#_glfs_open = ctypes.CFUNCTYPE(ctypes.c_void_p, ctypes.c_void_p, ctypes.c_char_p, ctypes.c_int) # noqa
-#                               (('glfs_open', client)) # noqa
 # TODO: # discard and fallocate fails with "AttributeError: /lib64/libgfapi.so.0: undefined symbol: glfs_discard", # noqa
 #  for time being, using it from api.* # noqa
 # glfs_discard = ctypes.CFUNCTYPE(ctypes.c_int, ctypes.c_void_p, ctypes.c_ulong, ctypes.c_size_t)(('glfs_discard', client)) # noqa
@@ -457,8 +463,6 @@ glfs_getcwd = ctypes.CFUNCTYPE(ctypes.c_char_p,
 #                                   (('glfs_fallocate', client)) # noqa
 
 
-#glfs_creat = ctypes.CFUNCTYPE(ctypes.c_void_p, ctypes.c_void_p, ctypes.c_char_p, ctypes.c_int, ctypes.c_uint)(('glfs_creat', client)) # noqa
-#glfs_open = ctypes.CFUNCTYPE(ctypes.c_void_p, ctypes.c_void_p, ctypes.c_char_p, ctypes.c_int)(('glfs_open', client)) # noqa
 
 #glfs_discard = ctypes.CFUNCTYPE(ctypes.c_int, ctypes.c_void_p, ctypes.c_ulong, ctypes.c_size_t)(('glfs_discard', client)) # noqa
 #glfs_fallocate = ctypes.CFUNCTYPE(ctypes.c_int, ctypes.c_void_p, ctypes.c_int, ctypes.c_ulong, ctypes.c_size_t)(('glfs_fallocate', client)) # noqa
diff --git a/gluster/gfapi.py b/gluster/gfapi.py
index 9b0d9a8..7fee1bb 100755
--- a/gluster/gfapi.py
+++ b/gluster/gfapi.py
@@ -777,9 +777,9 @@ class Volume(object):
             raise ValueError("Invalid mode")
         else:
             if (os.O_CREAT & flags) == os.O_CREAT:
-                fd = api.client.glfs_creat(self.fs, path, flags, 0666)
+                fd = api.glfs_creat(self.fs, path, flags, 0666)
             else:
-                fd = api.client.glfs_open(self.fs, path, flags)
+                fd = api.glfs_open(self.fs, path, flags)
             if not fd:
                 err = ctypes.get_errno()
                 raise OSError(err, os.strerror(err))
@@ -803,12 +803,9 @@ class Volume(object):
             raise TypeError("flags must evaluate to an integer")
 
         if (os.O_CREAT & flags) == os.O_CREAT:
-            # FIXME:
-            # Without direct call to _api the functest fails on creat and open.
-
-            fd = api.client.glfs_creat(self.fs, path, flags, mode)
+            fd = api.glfs_creat(self.fs, path, flags, mode)
         else:
-            fd = api.client.glfs_open(self.fs, path, flags)
+            fd = api.glfs_open(self.fs, path, flags)
         if not fd:
             err = ctypes.get_errno()
             raise OSError(err, os.strerror(err))
diff --git a/test/functional/libgfapi-python-tests.py b/test/functional/libgfapi-python-tests.py
index 13ae54d..87dbfe0 100644
--- a/test/functional/libgfapi-python-tests.py
+++ b/test/functional/libgfapi-python-tests.py
@@ -13,6 +13,7 @@ import unittest
 import os
 import types
 import errno
+import threading
 
 from gluster.gfapi import File, Volume
 from gluster.exceptions import LibgfapiException
@@ -197,6 +198,17 @@ class FileOpsTest(unittest.TestCase):
             f.lseek(0, os.SEEK_SET)
             self.assertEqual(f.read(), data + "hello world")
 
+    def test_fopen_in_thread(self):
+        def gluster_fopen():
+            name = uuid4().hex
+            with self.vol.fopen(name, 'w') as f:
+                f.write('hello world')
+
+        # the following caused segfault before the fix
+        thread = threading.Thread(target=gluster_fopen)
+        thread.start()
+        thread.join()
+
     def test_create_file_already_exists(self):
         try:
             f = File(self.vol.open("newfile", os.O_CREAT))
diff --git a/test/unit/gluster/test_gfapi.py b/test/unit/gluster/test_gfapi.py
index 7206aa1..d07ec67 100644
--- a/test/unit/gluster/test_gfapi.py
+++ b/test/unit/gluster/test_gfapi.py
@@ -440,7 +440,7 @@ class TestVolume(unittest.TestCase):
         mock_glfs_creat = Mock()
         mock_glfs_creat.return_value = 2
 
-        with patch("gluster.api.client.glfs_creat", mock_glfs_creat):
+        with patch("gluster.api.glfs_creat", mock_glfs_creat):
             with File(self.vol.open("file.txt", os.O_CREAT, 0644)) as f:
                 self.assertTrue(isinstance(f, File))
                 self.assertEqual(mock_glfs_creat.call_count, 1)
@@ -716,7 +716,7 @@ class TestVolume(unittest.TestCase):
         mock_glfs_open = Mock()
         mock_glfs_open.return_value = 2
 
-        with patch("gluster.api.client.glfs_open", mock_glfs_open):
+        with patch("gluster.api.glfs_open", mock_glfs_open):
             with File(self.vol.open("file.txt", os.O_WRONLY)) as f:
                 self.assertTrue(isinstance(f, File))
                 self.assertEqual(mock_glfs_open.call_count, 1)
@@ -731,14 +731,14 @@ class TestVolume(unittest.TestCase):
             with self.vol.open("file.txt", os.O_WRONLY) as fd:
                 self.assertEqual(fd, None)
 
-        with patch("gluster.api.client.glfs_open", mock_glfs_open):
+        with patch("gluster.api.glfs_open", mock_glfs_open):
             self.assertRaises(OSError, assert_open)
 
     def test_open_direct_success(self):
         mock_glfs_open = Mock()
         mock_glfs_open.return_value = 2
 
-        with patch("gluster.api.client.glfs_open", mock_glfs_open):
+        with patch("gluster.api.glfs_open", mock_glfs_open):
             f = File(self.vol.open("file.txt", os.O_WRONLY))
             self.assertTrue(isinstance(f, File))
             self.assertEqual(mock_glfs_open.call_count, 1)
@@ -749,7 +749,7 @@ class TestVolume(unittest.TestCase):
         mock_glfs_open = Mock()
         mock_glfs_open.return_value = None
 
-        with patch("gluster.api.client.glfs_open", mock_glfs_open):
+        with patch("gluster.api.glfs_open", mock_glfs_open):
             self.assertRaises(OSError, self.vol.open, "file.txt", os.O_RDONLY)
 
     def test_opendir_success(self):