[concurrency-interest] ConcurrentHashMap NullPointerException

BJ Low notorand at gmail.com
Sun Jan 6 20:19:59 EST 2008


On Jan 7, 2008 6:32 AM, David Holmes <dcholmes at optusnet.com.au> wrote:

>  BJ,
>
> I'm still missing two key parts to this:
>
> 1. As Endre asked, how do you know that Thread-1 has completed its action
>

class Upstream implements Runnable {

        private Logger logger = Logger.getLogger(Conn.Upstream.class);

        private ReentrantLock lockBuffer = new ReentrantLock();
        private ArrayDeque<EventIO> buffer = new ArrayDeque<EventIO>();

        private boolean streamInProc = false;
        private boolean eof = false;

        private Upstream() {
        }

        private void add(EventIO eventIO) {
            try {
                // prevent case where Event exist in buffer, but not
submitted to threadpool
                // - Thread_1, + IOHandlerThreadPool
                //
                // + run()
                // + if (this.buffer.size() != 0) test false
                // - add(EventIO eventIO)
                // - this.buffer.add(eventIO)
                // - if (!this.streamInProc) test false
                // + this.streamInProc = false
                // event in buffer but not submitted
                //
                // prevent concurrent access to buffer as ArrayDeque is not
thread-safe
                // - Thread_1, + IOHandlerThreadPool
                //
                // - add(EventIO eventIO)
                // - this.buffer.add(eventIO)
                // + run()
                // + eventIO = this.buffer.poll()
                this.lockBuffer.lock();

                // to prevent out-of-memory by not adding, within lockBuffer
to prevent memory consistency errors
                if (!this.eof) {

                    this.buffer.add(eventIO);

                    if (!this.streamInProc) {
                        Conn.this.ioHandlerThreadPool.submit(this);
                        this.streamInProc = true;
                    }

                    // CLOSE, CONNECT_FAIL will be the last event to be
processed, therefore any further add to the back add()/front close() of the
buffer is
                    // redundant
                    if ((eventIO == EventIO.CLOSE) || (eventIO ==
EventIO.CONNECT_FAIL)) {
                        this.eof = true;
                    }
                }
            }
            finally {
                this.lockBuffer.unlock();
            }
        }

        @Override
        public void run() {
            try {
                int granularity = 0;

                // stop delivery when granularity is reached, or no more
EventIO left in buffer to deliver
                boolean endLoop = false;
                boolean closed = false;
                while (!endLoop && !closed) {

                    EventIO eventIO = null;
                    try {
                        // prevent concurrent access to buffer as ArrayDeque
is not thread-safe
                        // - Thread_1, + IOHandlerThreadPool
                        //
                        // - add(EventIO eventIO)
                        // - this.buffer.add(eventIO)
                        // + run()
                        // + eventIO = this.buffer.poll()
                        this.lockBuffer.lock();

                        // remove an Object from buffer
                        eventIO = this.buffer.poll();
                    }
                    finally {
                        this.lockBuffer.unlock();
                    }

                    if (eventIO == null) {
                        endLoop = true;
                    }
                    else {
                        // prevent memory consistency errors whereby values
written by current thread from IOHandlerThreadPool is not flushed, causing
the next
                        // run() reading stale values if it is a different
thread from this current thread
                        synchronized (this) {

                            // delivery
                            if (eventIO == EventIO.OPEN) {
                                Conn.this.handleOpen();
                            }
                            else if (eventIO == EventIO.READ) {
                                Conn.this.handleRead();
                            }
                            else if ((eventIO == EventIO.IDLE_READ) ||
(eventIO == EventIO.IDLE_WRITE)) {
                                Conn.this.handleIdle(eventIO);
                            }
                            else if (eventIO == EventIO.CLOSE) {
                                Conn.this.handleClose();
                                closed = true;
                            }
                            else if (eventIO == EventIO.CONNECT_FAIL) {
                                Conn.this.handleConnectFail();
                                closed = true;
                            }
                        }

                        // update granularity
                        granularity++;
                        if (granularity == 10) {
                            endLoop = true;
                        }
                    }
                }

                // if EventIO.CLOSE is reached, then permanently
streamInProc stays true, ensuring no more EventIO will be delivered
                if (!closed) {
                    try {
                        // prevent case where Event exist in buffer, but not
submitted to threadpool
                        // - Thread_1, + IOHandlerThreadPool
                        //
                        // + run()
                        // + if (this.buffer.size() != 0) test false
                        // - add(EventIO eventIO)
                        // - this.buffer.add(eventIO)
                        // - if (!this.streamInProc) test false
                        // + this.streamInProc = false
                        // event in buffer but not submitted
                        this.lockBuffer.lock();

                        if (this.buffer.size() != 0) {
                            Conn.this.ioHandlerThreadPool.submit(this);
                        }
                        else {
                            this.streamInProc = false;
                        }
                    }
                    finally {
                        this.lockBuffer.unlock();
                    }
                }
            }
            catch (Throwable e) {
                // log FATAL, critical thread dies, node unable to function
                LoggerProxy.fatal(this.logger, "SYSTEM", "Died of unchecked
exception, node terminating", e);
                // fatal, terminate the program
                System.exit(1);
            }
        }
    }

As you can see above, EventIO is deposited into the buffer, and a thread
from threadpool is awakened to process the event, however it is only
possible for only 1 thread at any instance to do the run() method by using
the streamInProc boolean. Therefore there will only be 1 single thread
processing Upstream in any single instance


> 2. How is the instance of X accessed from two different Runnables
>
> Also, returning to the original issue, the use of the ConcurrentHashMap is
> significant here as well, as it also introduces happens-before
> relationships.
>
>
The thing is that during event delivery, thread-1 from threadpool might be
doing the run() method, and the next time round, thread-2 from threadpool
might be doing the run() method also. ConcurrentHashMap is accessed during
event delivery methods like Conn.this.handleOpen(), Conn.this.handleRead()
etc. There is this time when CHM is accessed, it returns null, when in fact
the element is put into the CHM already. It is a very rare occurrance. I
have run rigourous tests on the code day and night and it never happens,
there is only once when i am doing simple testing (under very insignifcant
load) that it occurs. I am not very sure if it is fixed, but it seems that
someone else has also encountered the same kind of bug
http://osdir.com/ml/java.jsr.166-concurrency/2003-12/msg00018.html


> So I still need more detail on the code to commemnt further.
>
> Cheers,
> David Holmes
>
> -----Original Message-----
> *From:* concurrency-interest-bounces at cs.oswego.edu [mailto:
> concurrency-interest-bounces at cs.oswego.edu]*On Behalf Of *BJ Low
> *Sent:* Sunday, 6 January 2008 12:15 AM
> *To:* Endre Stølsvik
> *Cc:* concurrency-interest at cs.oswego.edu; dholmes at ieee.org
> *Subject:* Re: [concurrency-interest] ConcurrentHashMap
> NullPointerException
>
> I think perhaps I never explain myself properly, Tim Peierls however did
> understood what i am trying to say.
>
> At time t1, Thread-X submits a Runnable to threadpool,
> At time t2, Thread-1 from the threadpool runs the Runnable and call
> changeVal(1);
>
> AFTER changeVal(1) from Thread-1 is completed, then at time t3 ....
>
> At time t3, Thread-X submits another Runnable to threadpool
> At time t4, Thread-2 from the threadpool runs the Runnable and call
> readVal();
>
> .... i missed out the red statement above. I think in above case, the
> happens-before property is very important, cause if at t3, it is Thread-1,
> but not Thread-X that submits the Runnable, then there will not be any
> memory consistency errors.
>
> On Jan 5, 2008 7:52 PM, Endre Stølsvik <Online at stolsvik.com> wrote:
>
> > BJ Low wrote:
> > > Hi,
> > >
> > > I am pretty sure of below. It is very hard for me to post the code
> > cause
> > > the code is proprietary, but perhaps let me ask this in another way
> > >
> > > Suppose I have a class below
> > >
> > > class X {
> > >      int i = 0;
> > >
> > >      void changeVal(int i) {
> > >         this.i = i;
> > >      }
> > >
> > >      int readVal() {
> > >         return this.i;
> > >      }
> > > }
> > >
> > > At time t1, Thread-X submits a Runnable to threadpool,
> > > At time t2, Thread-1 from the threadpool runs the Runnable and call
> > > changeVal(1);
> > > At time t3, Thread-X submits another Runnable to threadpool
> > > At time t4, Thread-2 from the threadpool runs the Runnable and call
> > > readVal();
> > >
> > > The question is, will Thread-2 see the value of i as 0 or 1?
> >
> > "Thread-X" - is that supposed to be the same thread at the two times?
> > Because I personally use "X" as "some".
> >
> > But lets assumes that (it doesn't really matter either - this is broken
> > no matter!).
> >
> > There's no reason to argue complex memory situations and happens-before
> > or anything like that here - this is plain as day: Thread X works on its
> > own, and thus basically submits two jobs right after each other. For the
> > sake of the argument: The first runnable, in Thread-1, takes 3 hours to
> > complete, finally doing changeVal(1). The second Runnable runs readVal()
> > about .001 ms after submit. What is the answer?
> >
> > And the time labels "t[1-4]" is of no use what so ever when interleaved
> > between threads that doesn't do explicit synchronization. Assuming again
> >
> > that Thread-X is the same in both instances, there is a "t, t+x"
> > situation between the two submits (the first submit /happens-before/ the
> > second submit, by way of code order).
> >   However, one cannot argue anything about Thread-1 (other than that it
> > happens AFTER the submit of itself, which is at "t"), and Thread-2
> > (other than that is happens AFTER the submit of itself, which is at
> > "t+x").
> >   The order of _when_ those two runnables are run on their respective
> > worker threads, is fully non-deterministic: both the work they do (the
> > time it takes), the inner workings of the thread pool (e.g. whether a
> > new Thread has to be made for one of the submissions, perhaps the first,
> >
> > not the second) - and finally, the operating system: it might decide
> > based on some cosmic reasons, that Thread-1 shouldn't run, or is
> > pre-empted before the changeVal() while a whole bunch of other threads
> > gets time slices, while Thread-2 by the same unfair magic gets scheduled
> > right away.
> >
> > These are really basic multi-threading issues, not having anything to do
> > with complex code-reorderings or anything like that.
> >
> > What you want here, is synchronized code in class X, and some flag that
> > denotes whether the job is done. The flag is set to true when
> > "changeVal()" is run, and then the /this X/ is .notify()ed (within
> > synchronized on /this X/, of course). The readVal() method checks
> > (within synchronzied on /this X/, of course) whether the flag is true.
> > If it isn't it goes into .wait() on /this X/, looping until flag is true
> >
> > (must loop due to "spurious wakeups"). Hey presto.. Also check out
> > Callable vs. Future - it does just this.
> >
> > Endre.
> >
>
>
>
> --
> Regards,
> BJ Low
>
>


-- 
Regards,
BJ Low
-------------- next part --------------
An HTML attachment was scrubbed...
URL: /pipermail/attachments/20080107/79aa3aa7/attachment-0001.html 


More information about the Concurrency-interest mailing list