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 <bur...@spinn3r.com> 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>
>
