Repository: nvim-lua/plenary.nvim
Pull request #133 (2021-04-10 12:28:27)

fix: semaphore counting
Fix counting semaphore around blocking threshold.
Add some test cases to cover basic behavior around blocking threshold.

cc @oberblastmeister

oberblastmeister: I didn't have the chance to test the semaphore so thanks for the help!
oberblastmeister: You should also probably add a test where the the permit is forgotten while someone is waiting on it, not just before someone is waiting for it so that they automatically get it.
RianFuro: > You should also probably add a test where the the permit is forgotten while someone is waiting on it, not just before someone is waiting for it so that they automatically get it.

That's what I attempted with the last test actually. Good catch. I updated the test and added another one that just tests sequential acquisition.

Patch:
diff --git a/lua/plenary/async_lib/util.lua b/lua/plenary/async_lib/util.lua
index b0d035c..d719c7d 100644
--- a/lua/plenary/async_lib/util.lua
+++ b/lua/plenary/async_lib/util.lua
@@ -127,9 +127,9 @@ end
 ---when a permit can be acquired returns it
 ---call permit:forget() to forget the permit
 Semaphore.acquire = a.wrap(function(self, callback)
-  self.permits = self.permits - 1
-
-  if self.permits <= 0 then
+  if self.permits > 0 then
+    self.permits = self.permits - 1
+  else
     table.insert(self.handles, callback)
     return
   end
@@ -140,9 +140,9 @@ Semaphore.acquire = a.wrap(function(self, callback)
     self.permits = self.permits + 1
 
     if self.permits > 0 and #self.handles > 0 then
+      self.permits = self.permits - 1
       local callback = table.remove(self.handles)
       callback(self_permit)
-      self.permits = self.permits - 1
     end
   end
 


Test patch:
diff --git a/tests/plenary/async_lib/semaphore_spec.lua b/tests/plenary/async_lib/semaphore_spec.lua
index f2908fb..f3df6a4 100644
--- a/tests/plenary/async_lib/semaphore_spec.lua
+++ b/tests/plenary/async_lib/semaphore_spec.lua
@@ -12,6 +12,47 @@ a.describe('semaphore', function()
     eq(status, false)
   end)
 
-  a.it('should count properly', function()
+  a.it('should acquire a permit if available', function ()
+    local sem = a.util.Semaphore.new(1)
+    local permit = await(sem:acquire())
+    assert(permit ~= nil)
+  end)
+
+  a.it('should block if no permit is available', function ()
+    local sem = a.util.Semaphore.new(1)
+    await(sem:acquire())
+    
+    local completed = false
+    local blocking = async(function ()
+      await(sem:acquire())
+      completed = true
+    end)
+    a.run(blocking())
+
+    eq(completed, false)
+  end)
+
+  a.it('should give another permit when an acquired permit is released', function ()
+    local sem = a.util.Semaphore.new(1)
+    local permit = await(sem:acquire())
+    permit:forget()
+    local next_permit = await(sem:acquire())
+    assert(next_permit ~= nil)
+  end)
+
+  a.it('should permit the next waiting client when a permit is released', function () 
+    local sem = a.util.Semaphore.new(1)
+    local permit = await(sem:acquire())
+
+    local completed = false
+    local blocking = async(function ()
+      await(sem:acquire())
+      completed = true
+    end)
+
+    a.run(blocking())
+    permit:forget()
+
+    eq(completed, true)
   end)
 end)