Merge "Fix open/fopen in thread other than main"

4 files changed, 33 insertions, 20 deletions

diff --git a/gluster/api.py b/gluster/api.py
index d2a0840..0ec5413 100755
--- a/gluster/api.py
+++ b/gluster/api.py
@@ -284,6 +284,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))
@@ -457,12 +469,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
@@ -470,8 +476,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):