Repository: tomMoral/loky

Issue:
loky.backend.semaphore_tracker.sem_unlink does not have same signature if coming from ctypes or _multiprocessing
* `_multi_processing.sem_unlink` takes `str`
* `loky.backend.semlock.sem_unlink` comes from `ctypes` and take `bytes`.

It feels like some code was written with the ctypes variant in mind and raise an error when the `_multiprocessing.sem_unlink` is called. Tests seem to be only testing `loky.backend.semlock.sem_unlink`.

#### Context
This is an error I just saw in a joblib Travis [build](https://travis-ci.org/joblib/joblib/jobs/346847911#L4044). Note this is with loky version 1.2.1.

```
E               /home/travis/build/joblib/joblib/joblib/externals/loky/backend/semaphore_tracker.py:195: UserWarning: semaphore_tracker: There appear to be 6 leaked semaphores to clean up at shutdown
E                 len(cache))
E               /home/travis/build/joblib/joblib/joblib/externals/loky/backend/semaphore_tracker.py:211: UserWarning: semaphore_tracker: b'/loky-5456-6haleho6': TypeError('argument 1 must be str, not bytes',)
E                 warnings.warn('semaphore_tracker: %r: %r' % (name, e)) 
```
Quickly looking at it, it seems like this is still in master. The code where the warning happens is here:
 https://github.com/tomMoral/loky/blob/dec1c8144b12938dfe7bfc511009e12f25fd1cd9/loky/backend/semaphore_tracker.py#L203-L211



Discussion:
ogrisel: > For the future, should we do the same as in joblib and run the test of joblib as part of the CI?

I agree but let's do that in another PR.

Patch:
diff --git a/loky/backend/semaphore_tracker.py b/loky/backend/semaphore_tracker.py
index 79587f2..f494237 100644
--- a/loky/backend/semaphore_tracker.py
+++ b/loky/backend/semaphore_tracker.py
@@ -203,7 +203,6 @@ def main(fd):
                 try:
                     sem_unlink(name)
                     if VERBOSE:  # pragma: no cover
-                        name = name.decode('ascii')
                         sys.stderr.write("[SemaphoreTracker] unlink {}\n"
                                          .format(name))
                         sys.stderr.flush()
diff --git a/loky/backend/semlock.py b/loky/backend/semlock.py
index c94c4cd..2d35f6a 100644
--- a/loky/backend/semlock.py
+++ b/loky/backend/semlock.py
@@ -68,7 +68,7 @@ if sys.version_info[:2] < (3, 3):
 
 
 def sem_unlink(name):
-    if pthread.sem_unlink(name) < 0:
+    if pthread.sem_unlink(name.encode('ascii')) < 0:
         raiseFromErrno()
 
 
@@ -153,8 +153,8 @@ class SemLock(object):
         self.ident = 0
         self.kind = kind
         self.maxvalue = maxvalue
-        self.name = name.encode('ascii')
-        self.handle = _sem_open(self.name, value)
+        self.name = name
+        self.handle = _sem_open(self.name.encode('ascii'), value)
 
     def __del__(self):
         try:
@@ -265,7 +265,7 @@ class SemLock(object):
         self.kind = kind
         self.maxvalue = maxvalue
         self.name = name
-        self.handle = _sem_open(name)
+        self.handle = _sem_open(name.encode('ascii'))
         return self
 
 
diff --git a/loky/backend/synchronize.py b/loky/backend/synchronize.py
index 2cdb43d..4773b9d 100644
--- a/loky/backend/synchronize.py
+++ b/loky/backend/synchronize.py
@@ -121,8 +121,7 @@ class SemLock(object):
     @staticmethod
     def _make_name():
         # OSX does not support long names for semaphores
-        name = '/loky-%i-%s' % (os.getpid(), next(SemLock._rand))
-        return name
+        return '/loky-%i-%s' % (os.getpid(), next(SemLock._rand))
 
 
 #


Test patch:
diff --git a/tests/test_synchronize.py b/tests/test_synchronize.py
index 797070d..4794f17 100644
--- a/tests/test_synchronize.py
+++ b/tests/test_synchronize.py
@@ -22,7 +22,7 @@ if sys.version_info < (3, 3):
 @pytest.mark.skipif(sys.platform == "win32", reason="UNIX test")
 def test_semlock_failure():
     from loky.backend.semlock import SemLock, sem_unlink
-    name = "test1"
+    name = "loky-test-semlock"
     sl = SemLock(0, 1, 1, name=name)
 
     with pytest.raises(FileExistsError):
@@ -30,7 +30,7 @@ def test_semlock_failure():
     sem_unlink(sl.name)
 
     with pytest.raises(FileNotFoundError):
-        SemLock._rebuild(None, 0, 0, name.encode('ascii'))
+        SemLock._rebuild(None, 0, 0, name)
 
 
 def assert_sem_value_equal(sem, value):