fix(semaphore) disable timeout on destroyed semaphore

fixes #118

docs/reference.html

@@ -359,7 +359,7 @@ <h3>Non-blocking data exchange and timer/sleep functions</h3>
     <dd>Creates and returns a new semaphore (fifo).
     <code>max</code> specifies the maximum number of resources the semaphore can hold.
     The optional <code>start</code> parameter (default 0) specifies the number of resources upon creation.
-    The <code>timeout</code> specifies the default timeout for the lock in
+    The <code>timeout</code> specifies the default timeout for the semaphore in
     seconds, and defaults to 10.
     </dd>
 

src/copas/semaphore.lua

@@ -12,8 +12,8 @@ local registry = setmetatable({}, { __mode="kv" })
 
 -- create a new semaphore
 -- @param max maximum number of resources the semaphore can hold (this maximum does NOT include resources that have been given but not yet returned).
--- @param seconds (optional, default 10) default semaphore timeout in seconds
 -- @param start (optional, default 0) the initial resources available
+-- @param seconds (optional, default 10) default semaphore timeout in seconds
 function semaphore.new(max, start, seconds)
   local timeout = tonumber(seconds or DEFAULT_TIMEOUT) or -1
   if timeout < 0 then
@@ -172,12 +172,14 @@ function semaphore:take(requested, timeout)
     -- a timeout happened
     self.to_flags[co] = nil
     return nil, "timeout"
-  elseif self.destroyed then
-    return nil, "destroyed"
   end
 
   copas.timeout(0)
 
+  if self.destroyed then
+    return nil, "destroyed"
+  end
+
   return true
 end
 

tests/semaphore.lua

@@ -95,14 +95,51 @@ copas.loop(function()
   copas.sleep(0.1)
   assert(sema:destroy())
   copas.sleep(0.1)
-  assert(state == 2, "expected 2 thread to error with 'destroyed'")
+  assert(state == 2, "expected 2 threads to error with 'destroyed'")
 
   -- only returns errors from now on, on all methods
   ok, err = sema:destroy();   assert(ok == nil and err == "destroyed", "expected an error")
   ok, err = sema:give(1);     assert(ok == nil and err == "destroyed", "expected an error")
   ok, err = sema:take(1);     assert(ok == nil and err == "destroyed", "expected an error")
   ok, err = sema:get_count(); assert(ok == nil and err == "destroyed", "expected an error")
 
+
+
+  -- timeouts get cancelled upon destruction
+  -- we set a timeout to 0.5 seconds, then destroy the semaphore
+  -- the timeout should not execute
+  -- Reproduce https://github.com/lunarmodules/copas/issues/118
+  local track_table = setmetatable({}, { __mode = "v" })
+  local sema = semaphore.new(10, 0, 0.5)
+  track_table.sema = sema
+  local state = 0
+  track_table.coro = copas.addthread(function()
+    local ok, err = sema:take(1)
+    if ok then
+      print("got one, this is unexpected")
+    elseif err == "destroyed" then
+      state = state + 1
+    end
+  end)
+  copas.sleep(0.1)
+  assert(sema:destroy())
+  copas.sleep(0.1)
+  assert(state == 1, "expected 1 thread to error with 'destroyed'")
+  sema = nil
+
+  local errors = 0
+  copas.setErrorHandler(function(msg)
+    print("got error: "..tostring(msg))
+    print("--------------------------------------")
+    errors = errors + 1
+  end, true)
+
+  collectgarbage()  -- collect garbage to force eviction from the semaphore registry
+  collectgarbage()
+
+  copas.sleep(0.5) -- wait for the timeout to expire if it is still set
+  assert(errors == 0, "expected no errors")
+
   test_complete = true
 end)
 assert(test_complete, "test did not complete!")