I didn’t.. it’s a race so if it’s happening is probably happening not that often . Maybe a test could be created to try to introduce the race.
On Thu, Jun 18, 2015 at 9:57 PM, Tim Bain <[email protected]> wrote: > Did you look at this any further? Looking at the code, it looks like the > call will be protected without explicit synchronization by the intrinsic > lock on the synchronizedMap (and I think that some other methods such as > delete() and addMessage() that just call a method on the synchronizedMap > could have their synchronized blocks removed), though I might be looking at > that wrong. > > Tim > > On Mon, Apr 6, 2015 at 1:58 PM, Kevin Burton <[email protected]> wrote: > > > Pretty sure getMessage() in MemoryMessageStore has a bug. > > > > All access to messageTable is synchronized. this method is not. This > > means that there’s a race where a message can go into the queue but the > > thread reading it may have a cache copy of the data structure meaning it > > would get a cache miss > > > > Also, it looks like “addMessage” is doubly synchronized. > > > > public Message getMessage(MessageId identity) throws IOException { > > return messageTable.get(identity); > > } > > > > … I’m going to migrate to using a PriorityBlockingQueue for this and > remove > > all the synchronization and will try to submit a patch. Also I think > > PriorityBlockingQueue will lower memory usage by 40% > > > > > > -- > > > > Founder/CEO Spinn3r.com > > Location: *San Francisco, CA* > > blog: http://burtonator.wordpress.com > > … or check out my Google+ profile > > <https://plus.google.com/102718274791889610666/posts> > > <http://spinn3r.com> > > > -- Founder/CEO Spinn3r.com Location: *San Francisco, CA* blog: http://burtonator.wordpress.com … or check out my Google+ profile <https://plus.google.com/102718274791889610666/posts>
