From mboxrd@z Thu Jan 1 00:00:00 1970 From: ludo@gnu.org (Ludovic =?UTF-8?Q?Court=C3=A8s?=) Subject: bug#28779: tests/workers.scm failure Date: Thu, 16 Nov 2017 09:29:38 +0100 Message-ID: <87shdelj7x.fsf@gnu.org> References: Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:51412) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eFFYm-0005ek-JH for bug-guix@gnu.org; Thu, 16 Nov 2017 03:30:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eFFYi-0003qj-Jz for bug-guix@gnu.org; Thu, 16 Nov 2017 03:30:08 -0500 Received: from debbugs.gnu.org ([208.118.235.43]:33837) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1eFFYi-0003qb-Fe for bug-guix@gnu.org; Thu, 16 Nov 2017 03:30:04 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1eFFYi-0006jl-2b for bug-guix@gnu.org; Thu, 16 Nov 2017 03:30:04 -0500 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: (Eric Bavier's message of "Tue, 10 Oct 2017 15:48:42 +0000") List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: Eric Bavier Cc: 28779@debbugs.gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi Eric, Eric Bavier skribis: > test-name: enqueue > location: /home/users/bavier/src/guix/tests/workers.scm:26 > source: > + (test-equal > + "enqueue" > + 4242 > + (let* ((pool (make-pool)) > + (result 0) > + (#{1+!}# (let ((lock (make-mutex))) > + (lambda () > + (with-mutex lock (set! result (+ result 1))))))) > + (let loop ((i 4242)) > + (unless > + (zero? i) > + (pool-enqueue! pool #{1+!}#) > + (loop (- i 1)))) > + (let poll () > + (unless > + (pool-idle? pool) > + (pk 'busy result) > + (sleep 1) > + (poll))) > + result)) > expected-value: 4242 > actual-value: 4241 > result: FAIL > > > To me the reason seems to be that the 'pool-idle? procedure indicates whe= ther or not the task queue is empty, not whether all tasks have completed e= xecution, so the poll loop exits before all 1+! updates are finished and th= e test fails.=20=20 Indeed, good catch. The attached patch is a bit crude but it should fix the problem. Thoughts? Thanks, Ludo=E2=80=99. --=-=-= Content-Type: text/x-patch Content-Disposition: inline diff --git a/guix/workers.scm b/guix/workers.scm index 846f5e50a..0f6f54bab 100644 --- a/guix/workers.scm +++ b/guix/workers.scm @@ -45,12 +45,13 @@ ;;; Code: (define-record-type - (%make-pool queue mutex condvar workers) + (%make-pool queue mutex condvar workers busy) pool? (queue pool-queue) (mutex pool-mutex) (condvar pool-condition-variable) - (workers pool-workers)) + (workers pool-workers) + (busy pool-busy)) (define-syntax-rule (without-mutex mutex exp ...) (dynamic-wind @@ -62,12 +63,14 @@ (lock-mutex mutex)))) (define* (worker-thunk mutex condvar pop-queue - #:key (thread-name "guix worker")) + #:key idle busy (thread-name "guix worker")) "Return the thunk executed by worker threads." (define (loop) (match (pop-queue) (#f ;empty queue - (wait-condition-variable condvar mutex)) + (idle) + (wait-condition-variable condvar mutex) + (busy)) ((? procedure? proc) ;; Release MUTEX while executing PROC. (without-mutex mutex @@ -97,19 +100,24 @@ threads as reported by the operating system." (let* ((mutex (make-mutex)) (condvar (make-condition-variable)) (queue (make-q)) + (busy count) (procs (unfold (cut >= <> count) (lambda (n) (worker-thunk mutex condvar (lambda () (and (not (q-empty? queue)) (q-pop! queue))) + #:busy (lambda () + (set! busy (+ 1 busy))) + #:idle (lambda () + (set! busy (- busy 1))) #:thread-name thread-name)) 1+ 0)) (threads (map (lambda (proc) (call-with-new-thread proc)) procs))) - (%make-pool queue mutex condvar threads))) + (%make-pool queue mutex condvar threads (lambda () busy)))) (define (pool-enqueue! pool thunk) "Enqueue THUNK for future execution by POOL." @@ -118,9 +126,11 @@ threads as reported by the operating system." (signal-condition-variable (pool-condition-variable pool)))) (define (pool-idle? pool) - "Return true if POOL doesn't have any task in its queue." + "Return true if POOL doesn't have any task in its queue and all the workers +are currently idle (i.e., waiting for a task)." (with-mutex (pool-mutex pool) - (q-empty? (pool-queue pool)))) + (and (q-empty? (pool-queue pool)) + (zero? ((pool-busy pool)))))) (define-syntax-rule (eventually pool exp ...) "Run EXP eventually on one of the workers of POOL." --=-=-=--