[j-devel] Mailbox::read

[Alex M. <kil...@ne...>]

Hello, All!

Mailbox::read has non-synchronized box.removeFirst:

    private LispObject read ()
    {
        while (box.isEmpty())
            synchronized(this) {
                try {
                    wait();
                } catch(InterruptedException e) {
                    throw new RuntimeException(e);
                }
            }
        return (LispObject) box.removeFirst();
    }

that means that if multiple threads are waiting in ext:mailbox-read 
simultaneously, they'll get same objects.
i'd suggest to make it synchornized:

    private LispObject read ()
    {
     for (;;)
     synchronized (this) {
      if (box.isEmpty()) {
       try {
        wait();
       } catch(InterruptedException e) {  throw new RuntimeException(e); }
      } else {
       return (LispObject) box.removeFirst();
      }
     }
    }

although multiple reader threads might be not original intent, that allows 
to create a pool of worker threads.

for example:

(defun exec-parallel (work-list worker-count)
  (let* ((in-mbox (ext:make-mailbox))
  (out-mbox (ext:make-mailbox))
  (thread-fun (lambda ()
         (loop for w = (ext:mailbox-read in-mbox)
    while w ;;exit when nil sent
    do (ext:mailbox-send out-mbox (funcall w))))))
    (loop for work in work-list do (ext:mailbox-send in-mbox work))
    (loop  for i from 1 to worker-count
      do (ext:make-thread thread-fun)
      do (ext:mailbox-send in-mbox nil))
    (loop repeat (length work-list)
      collect (ext:mailbox-read out-mbox))))

(exec-parallel (loop for i from 1 to 20
                 collect (let ((i i)) (lambda  () (print i))))
   4)

with my fix that makes correct result -- no numbers get duplicated, while 
they were duplicated with the original code.

With best regards, Alex Mizrahi.