[mkgmap-dev] NPE in splitter crosby_integration branch

Sat Feb 19 15:55:18 GMT 2011

On 17/02/11 20:25, Jeffrey Ollie wrote:
> I think that I've tracked down an NPE in the crosby_integration branch
> of the splitter.  The SplitProcessor is using a BlockingQueue to queue
> up elements for writing.  The poll() method can potentially return a
> null if the queue is empty.  My question is, should the poll() method
> call be switched to a take() method call - the take() method will
> block if the queue is empty, or should I guard the usage of elements
> for null like in the patch below?

I think that if you get a null then you should break out of the loop
so that the thread can pick up the next work package.  By waiting or
spinning while the workPackage is empty, the thread wastes time (and I
don't suppose there is any guarantee that anything will ever be added).

But lets look at what is happening, this is my take:

The synchronized line does nothing useful so it can be removed.

Then there is a check that the queue is not empty.

Later we try to read from it and find that it is empty. This means
that some other thread is also reading from the same queue.  From the
code this is possible, but possibly unintended - I would expect that
the reason that the work is parcelled up in the first place is to
avoid excessive contention reading work items by different threads
from a single queue.

In any case, the check for empty is also useless and so can be
removed. The remaining code would then look like this:

	ArrayList<Element> elements;
	while ((elements = workPackage.inputQueue.poll()) != null) {
		try {
			for (Element element : elements ) {
				processElement(element, workPackage.writer);
			}
		} catch (IOException e) {
			throw new RuntimeException("Thread " + 
Thread.currentThread().getName() + " failed to write element ", e);
		}
	}

Also attached as a patch, could you try it please.

Oh and thanks for looking into these problems, I'm going to look
at your second patch now.

Best wishes
..Steve
-------------- next part --------------
A non-text attachment was scrubbed...
Name: split_npe.patch
Type: text/x-patch
Size: 1322 bytes
Desc: not available
Url : http://lists.mkgmap.org.uk/pipermail/mkgmap-dev/attachments/20110219/57284dc0/attachment.bin 