[ 
https://issues.apache.org/jira/browse/QPID-2565?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Keith Wall updated QPID-2565:
-----------------------------

    Description: 
Summary:

The Topic exchange may generate an NPE during the message routing process, as 
seen in stack trace (comment below). It is believed this can only be attributed 
to 'undefined behaviour' defined in the map.Entry Javadoc, which may be 
exhibited by Map.Entry objects returned from an Iterator over the keySet() and 
entrySet() results of a ConcurrentHashMap in the event the map is updated 
directly following generation of the Map.Entry. The code section in question 
should be updated to remove use of Map.Entry since its behaviour in this case 
cannot be guaranteed otherwise without additional locking that would hinder 
performance of the exchange.


Additional Detail:

It hasnt been possible to reproduce the error thus far, and at first glance it 
was believed the code in question should never be able to NPE, as none of the 
methods should be able to return null based on the fact they either return a 
new Set() or a key/value that can never be null, as null can never be entered 
into the map. 

However, searching the Sun JDK BugDatabase revealed a historical instance ( 
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6312056 ) where the 
Map.Entry.value() did return null, and although this case was fixed the report 
also draws attention to the fact that usage of Map.Entry's returned by the 
Iterator from a ConcurrentHashMap are not completely safe, and may exhibit 
'undefined behaviour' if the Map is modified thereafter via any means other 
than the map.Entry itself, despite the fact the Iterator itself is safe in such 
cases. 

http://java.sun.com/j2se/1.5.0/docs/api/java/util/Map.Entry.html states:
"These Map.Entry objects are valid only for the duration of the iteration; more 
formally, the behavior of a map entry is undefined if the backing map has been 
modified after the entry was returned by the iterator, except through the 
setValue operation on the map entry. "


The NPE occurs on the last line of the following code section:

for(Map.Entry<AMQQueue, Map<MessageFilter<RuntimeException>, Integer>> entry : 
_filteredQueues.entrySet())
{
    if(!queues.contains(entry.getKey()))
    {
         for(MessageFilter<RuntimeException> filter : entry.getValue().keySet())


The _filteredQueues map is a ConcurrentHashMap, itself containing 
ConcurrentHashMap values. It is not possible for a null to be used as either 
key or value in a CHM, and both keyset() or entrySet() return a new Set if none 
previously existed. As a result it would seem that it should not be possible 
for the above line to ever contain a null unless due to the aforementioned 
undefined behaviour.


  was:
Summary:

The Topic exchange may generate an NPE during the message routing process, as 
seen in IBAMQ-1344. It is believed this can only be attributed to 'undefined 
behaviour' defined in the map.Entry Javadoc, which may be exhibited by 
Map.Entry objects returned from an Iterator over the keySet() and entrySet() 
results of a ConcurrentHashMap in the event the map is updated directly 
following generation of the Map.Entry. The code section in question should be 
updated to remove use of Map.Entry since its behaviour in this case cannot be 
guaranteed otherwise without additional locking that would hinder performance 
of the exchange.


Additional Detail:

It hasnt been possible to reproduce the error thus far, and at first glance it 
was believed the code in question should never be able to NPE, as none of the 
methods should be able to return null based on the fact they either return a 
new Set() or a key/value that can never be null, as null can never be entered 
into the map. 

However, searching the Sun JDK BugDatabase revealed a historical instance ( 
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6312056 ) where the 
Map.Entry.value() did return null, and although this case was fixed the report 
also draws attention to the fact that usage of Map.Entry's returned by the 
Iterator from a ConcurrentHashMap are not completely safe, and may exhibit 
'undefined behaviour' if the Map is modified thereafter via any means other 
than the map.Entry itself, despite the fact the Iterator itself is safe in such 
cases. 

http://java.sun.com/j2se/1.5.0/docs/api/java/util/Map.Entry.html states:
"These Map.Entry objects are valid only for the duration of the iteration; more 
formally, the behavior of a map entry is undefined if the backing map has been 
modified after the entry was returned by the iterator, except through the 
setValue operation on the map entry. "


The NPE occurs on the last line of the following code section:

for(Map.Entry<AMQQueue, Map<MessageFilter<RuntimeException>, Integer>> entry : 
_filteredQueues.entrySet())
{
    if(!queues.contains(entry.getKey()))
    {
         for(MessageFilter<RuntimeException> filter : entry.getValue().keySet())


The _filteredQueues map is a ConcurrentHashMap, itself containing 
ConcurrentHashMap values. It is not possible for a null to be used as either 
key or value in a CHM, and both keyset() or entrySet() return a new Set if none 
previously existed. As a result it would seem that it should not be possible 
for the above line to ever contain a null unless due to the aforementioned 
undefined behaviour.



> Topic exchange may NPE during message routing
> ---------------------------------------------
>
>                 Key: QPID-2565
>                 URL: https://issues.apache.org/jira/browse/QPID-2565
>             Project: Qpid
>          Issue Type: Bug
>    Affects Versions: M4, 0.5, 0.6, 0.7
>            Reporter: Robbie Gemmell
>
> Summary:
> The Topic exchange may generate an NPE during the message routing process, as 
> seen in stack trace (comment below). It is believed this can only be 
> attributed to 'undefined behaviour' defined in the map.Entry Javadoc, which 
> may be exhibited by Map.Entry objects returned from an Iterator over the 
> keySet() and entrySet() results of a ConcurrentHashMap in the event the map 
> is updated directly following generation of the Map.Entry. The code section 
> in question should be updated to remove use of Map.Entry since its behaviour 
> in this case cannot be guaranteed otherwise without additional locking that 
> would hinder performance of the exchange.
> Additional Detail:
> It hasnt been possible to reproduce the error thus far, and at first glance 
> it was believed the code in question should never be able to NPE, as none of 
> the methods should be able to return null based on the fact they either 
> return a new Set() or a key/value that can never be null, as null can never 
> be entered into the map. 
> However, searching the Sun JDK BugDatabase revealed a historical instance ( 
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6312056 ) where the 
> Map.Entry.value() did return null, and although this case was fixed the 
> report also draws attention to the fact that usage of Map.Entry's returned by 
> the Iterator from a ConcurrentHashMap are not completely safe, and may 
> exhibit 'undefined behaviour' if the Map is modified thereafter via any means 
> other than the map.Entry itself, despite the fact the Iterator itself is safe 
> in such cases. 
> http://java.sun.com/j2se/1.5.0/docs/api/java/util/Map.Entry.html states:
> "These Map.Entry objects are valid only for the duration of the iteration; 
> more formally, the behavior of a map entry is undefined if the backing map 
> has been modified after the entry was returned by the iterator, except 
> through the setValue operation on the map entry. "
> The NPE occurs on the last line of the following code section:
> for(Map.Entry<AMQQueue, Map<MessageFilter<RuntimeException>, Integer>> entry 
> : _filteredQueues.entrySet())
> {
>     if(!queues.contains(entry.getKey()))
>     {
>          for(MessageFilter<RuntimeException> filter : 
> entry.getValue().keySet())
> The _filteredQueues map is a ConcurrentHashMap, itself containing 
> ConcurrentHashMap values. It is not possible for a null to be used as either 
> key or value in a CHM, and both keyset() or entrySet() return a new Set if 
> none previously existed. As a result it would seem that it should not be 
> possible for the above line to ever contain a null unless due to the 
> aforementioned undefined behaviour.

-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:[email protected]

Reply via email to