[Jython-checkins] jython (2.5): #1676 fixed NPE in defaultdict. Thanks Geoffrey French for initial patch!

[frank.wierzbicki]

http://hg.python.org/jython/rev/9124160150ae
changeset:   7003:9124160150ae
branch:      2.5
parent:      7001:328c4e80117f
user:        Frank Wierzbicki <fwierzbicki at gmail.com>
date:        Wed Feb 06 14:53:22 2013 -0800
summary:
  #1676 fixed NPE in defaultdict. Thanks Geoffrey French for initial patch!

files:
  Lib/test/test_dict_jy.py              |   7 ++++
  NEWS                                  |   1 +
  src/org/python/core/PyDictionary.java |  23 ++++++++------
  3 files changed, 21 insertions(+), 10 deletions(-)


diff --git a/Lib/test/test_dict_jy.py b/Lib/test/test_dict_jy.py
--- a/Lib/test/test_dict_jy.py
+++ b/Lib/test/test_dict_jy.py
@@ -1,6 +1,7 @@
 from test import test_support
 import java
 import unittest
+from collections import defaultdict
 
 class DictInitTest(unittest.TestCase):
     def testInternalSetitemInInit(self):
@@ -93,6 +94,12 @@
             def __getitem__(self, key):
                 raise CustomKeyError("custom message")
         self.assertRaises(CustomKeyError, lambda: DerivedDict()['foo'])
+    
+    def test_issue1676(self):
+        #See http://bugs.jython.org/issue1676
+        x=defaultdict()
+        #This formerly caused an NPE.
+        self.assertEqual(None, x.pop(None,None))
 
 class JavaIntegrationTest(unittest.TestCase):
     "Tests for instantiating dicts from Java maps and hashtables"
diff --git a/NEWS b/NEWS
--- a/NEWS
+++ b/NEWS
@@ -5,6 +5,7 @@
 
 Jython 2.5.4rc1
   Bugs Fixed
+    - [ 1676 ] NPE in defaultdict 
     - [ 1481 ] jython throws java.lang.IllegalArgumentException instead of ValueError.
     - [ 1716 ] xrange slicing raises NPE.
 
diff --git a/src/org/python/core/PyDictionary.java b/src/org/python/core/PyDictionary.java
--- a/src/org/python/core/PyDictionary.java
+++ b/src/org/python/core/PyDictionary.java
@@ -31,10 +31,10 @@
 
     public static final PyType TYPE = PyType.fromClass(PyDictionary.class);
 
-    private final ConcurrentMap<PyObject, PyObject> map;
+    private final ConcurrentMap<PyObject, PyObject> internalMap;
 
     public ConcurrentMap<PyObject, PyObject> getMap() {
-        return map;
+        return internalMap;
     }
 
     /**
@@ -49,8 +49,8 @@
      */
     public PyDictionary(PyType type, int capacity) {
         super(type);
-        map = new ConcurrentHashMap<PyObject, PyObject>(capacity, Generic.CHM_LOAD_FACTOR,
-                                                          Generic.CHM_CONCURRENCY_LEVEL);
+        internalMap = new ConcurrentHashMap<PyObject,PyObject>(capacity, Generic.CHM_LOAD_FACTOR,
+                                                               Generic.CHM_CONCURRENCY_LEVEL);
     }
 
     /**
@@ -58,7 +58,7 @@
      */
     public PyDictionary(PyType type) {
         super(type);
-        map = Generic.concurrentMap();
+        internalMap = Generic.concurrentMap();
     }
 
     /**
@@ -74,7 +74,7 @@
     public PyDictionary(PyType type, Map<PyObject, PyObject> map) {
         this(type, Math.max((int) (map.size() / Generic.CHM_LOAD_FACTOR) + 1,
                             Generic.CHM_INITIAL_CAPACITY));
-        this.map.putAll(map);
+        getMap().putAll(map);
     }
 
     /**
@@ -85,9 +85,9 @@
     protected PyDictionary(PyType type, boolean initializeBacking) {
         super(type);
         if (initializeBacking) {
-            map = Generic.concurrentMap();
+            internalMap = Generic.concurrentMap();
         } else {
-            map = null; // for later initialization
+            internalMap = null; // for later initialization
         }
     }
 
@@ -100,6 +100,7 @@
      */
     public PyDictionary(PyObject elements[]) {
         this();
+        ConcurrentMap<PyObject, PyObject> map = getMap();
         for (int i = 0; i < elements.length; i += 2) {
             map.put(elements[i], elements[i + 1]);
         }
@@ -599,7 +600,7 @@
 
     @ExposedMethod(defaults = "null", doc = BuiltinDocs.dict_pop_doc)
     final PyObject dict_pop(PyObject key, PyObject defaultValue) {
-        if (!map.containsKey(key)) {
+        if (!getMap().containsKey(key)) {
             if (defaultValue == null) {
                 throw Py.KeyError("popitem(): dictionary is empty");
             }
@@ -713,7 +714,9 @@
             return false;
         }
         final PyDictionary other = (PyDictionary) obj;
-        if (this.map != other.map && (this.map == null || !this.map.equals(other.map))) {
+        ConcurrentMap<PyObject, PyObject> map = getMap();
+        ConcurrentMap<PyObject, PyObject> otherMap = other.getMap();
+        if (map != otherMap && (map == null || !map.equals(otherMap))) {
             return false;
         }
         return true;

-- 
Repository URL: http://hg.python.org/jython