sionescu / bordeaux-threads

Portable shared-state concurrency for Common Lisp
http://common-lisp.net/project/bordeaux-threads/
MIT License
232 stars 47 forks source link

Add SEMAPHORE-COUNT #70

Open phoe opened 4 years ago

phoe commented 4 years ago

In the Lisp Koans, we have a threading koan that, when solved, is supposed to look similar to the following:

(defvar *semaphore* (bt:make-semaphore))

(defun signal-our-semaphore ()
  (bt:signal-semaphore *semaphore*))

(defun wait-on-our-semaphore ()
  (bt:wait-on-semaphore *semaphore* :timeout 0.1))

(define-test semaphore
  (bt:join-thread (bt:make-thread #'signal-our-semaphore))
  (bt:join-thread (bt:make-thread #'signal-our-semaphore))
  (bt:join-thread (bt:make-thread #'signal-our-semaphore))
  (assert-equal 2 (bt:join-thread (bt:make-thread #'wait-on-our-semaphore)))
  (assert-equal 1 (bt:join-thread (bt:make-thread #'wait-on-our-semaphore)))
  (assert-equal 0 (bt:join-thread (bt:make-thread #'wait-on-our-semaphore)))
  (assert-equal nil (bt:join-thread (bt:make-thread #'wait-on-our-semaphore)))
  (assert-equal nil (bt:join-thread (bt:make-thread #'wait-on-our-semaphore))))

The previous version of koan also had semaphore-count calls to assert the correct behaviour of the threading system. It looked similar to the following:

(define-test semaphore
  (assert-equal 0 (sb-thread:semaphore-count *semaphore*))
  (bt:join-thread (bt:make-thread #'signal-our-semaphore))
  (assert-equal 1 (sb-thread:semaphore-count *semaphore*))
  (bt:join-thread (bt:make-thread #'signal-our-semaphore))
  (bt:join-thread (bt:make-thread #'signal-our-semaphore))
  (assert-equal 3 (sb-thread:semaphore-count *semaphore*))
  (assert-equal 2 (bt:join-thread (bt:make-thread #'wait-on-our-semaphore)))
  (assert-equal 1 (bt:join-thread (bt:make-thread #'wait-on-our-semaphore)))
  (assert-equal 1 (sb-thread:semaphore-count *semaphore*))
  (assert-equal 0 (bt:join-thread (bt:make-thread #'wait-on-our-semaphore)))
  (assert-equal 0 (sb-thread:semaphore-count *semaphore*))
  (assert-equal nil (bt:join-thread (bt:make-thread #'wait-on-our-semaphore)))
  (assert-equal 0 (sb-thread:semaphore-count *semaphore*)))

The obvious missing thing is the lack of bt:semaphore-count in Bordeaux Threads. The Lisp Koans rolled out its own semaphore object based on the bordeaux-threads provided one to work around this issue, which I have removed in https://github.com/google/lisp-koans/pull/111 to clean up the code.


A comment from @Slids at https://github.com/Clozure/ccl/issues/308 says:

Stellian doesn't want a semaphore-count function, already asked. Arguably it's not very informative, the count could change immediately after or before retrieved and and since it's a condition variable you have no great use of it for a CAS utility...

I agree that such a function is nigh useless in actual production code, since the simple act of returning a semaphore value causes it to immediately become stale. Therefore depending on the value of this function in multithreaded code single-handedly enables race conditions to happen.

At the same time, the Lisp Koans have come up with, IMO, a somewhat surprising valid use case for that function - which is, introspecting the semaphore state in a controlled, single-threaded unit test case, where the semaphore is only available to the thread executing the test case. (The Koans spawn other threads via bt:make-thread and then wait for them to bt:join-thread, which is single-threaded enough, since the spawning thread is effectively paused at the time.) I imagine that other unit tests that single-threadedly run chunks of a multithreaded code to verify its functioning might also want to benefit from numeric assertions on the semaphore count during any given moment in the test execution.

Technically speaking, SBCL exports a function for accessing the count of a semaphore, the not-yet-merged ECL 20.4.24 semaphores also export one; it is also trivial to extract it from the current portable implementation that BT has for other CL implementations. The only missing implementation is CCL with a ticket at https://github.com/Clozure/ccl/issues/308, but it seems that adding a ccl:semaphore-count that works similarly should be easy and therefore wouldn't be a blocker for adding this function to BT. Therefore, I think it can be said that there is a semaphore-count API that a portability library can wrap over.

Therefore, my question is: since, IMO, a valid use case for it has appeared, would you consider adding BT:SEMAPHORE-COUNT with the aforementioned warning for people who dare use it in production code?

sionescu commented 4 years ago

See https://github.com/Clozure/ccl/issues/308#issuecomment-643615898.

phoe commented 4 years ago

OK - thanks for the reply. We'll implement the semaphore count function in CCL; it should be available in the next CCL release.

sionescu commented 4 years ago

When is the next release going to happen ?

phoe commented 4 years ago

I don't know about the release itself, but I still have not implemented this part; sorry, and thank you for the reminder.

sionescu commented 3 years ago

Any updates ?

phoe commented 3 years ago

I completely forgot about this issue and I do not have any progress in this matter or the related CCL ticket.