From 0d210a99c34ff64a6a960200279c495f9f90ca96 Mon Sep 17 00:00:00 2001
From: "Siyuan (Ryans) Zhuang" <suquark@gmail.com>
Date: Wed, 19 Feb 2020 23:30:10 -0800
Subject: [PATCH] Ensure deserialized numpy arrays are immutable (#7181)

* ensure numpy arrays are immutable when deserialized from the memory buffer
---
 python/ray/cloudpickle/cloudpickle_fast.py | 15 +++++++++++----
 python/ray/includes/serialization.pxi      |  8 +++++++-
 python/ray/tests/test_basic.py             |  9 +++++++++
 3 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/python/ray/cloudpickle/cloudpickle_fast.py b/python/ray/cloudpickle/cloudpickle_fast.py
index a0a5a44f6..75fd89e6b 100644
--- a/python/ray/cloudpickle/cloudpickle_fast.py
+++ b/python/ray/cloudpickle/cloudpickle_fast.py
@@ -411,6 +411,16 @@ def _property_reduce(obj):
     return property, (obj.fget, obj.fset, obj.fdel, obj.__doc__)
 
 
+def _numpy_frombuffer(buffer, dtype, shape, order):
+    # Get the _frombuffer() function for reconstruction
+    from numpy.core.numeric import _frombuffer
+    array = _frombuffer(buffer, dtype, shape, order)
+    # Unfortunately, numpy does not follow the standard, so we still
+    # have to set the readonly flag for it here.
+    array.setflags(write=not buffer.readonly)
+    return array
+
+
 def _numpy_ndarray_reduce(array):
     # This function is implemented according to 'array_reduce_ex_picklebuffer'
     # in numpy C backend. This is a workaround for python3.5 pickling support.
@@ -443,10 +453,7 @@ def _numpy_ndarray_reduce(array):
         # (gh-12745).
         return array.__reduce__()
 
-    # Get the _frombuffer() function for reconstruction
-    import numpy.core.numeric as numeric_mod
-    from_buffer_func = numeric_mod._frombuffer
-    return from_buffer_func, (buffer, array.dtype, array.shape, order)
+    return _numpy_frombuffer, (buffer, array.dtype, array.shape, order)
 
 
 class CloudPickler(Pickler):
diff --git a/python/ray/includes/serialization.pxi b/python/ray/includes/serialization.pxi
index c86414da0..ce5fca653 100644
--- a/python/ray/includes/serialization.pxi
+++ b/python/ray/includes/serialization.pxi
@@ -98,6 +98,10 @@ cdef class SubBuffer:
         """
         return self.len
 
+    @property
+    def readonly(self):
+        return self.readonly
+
     def tobytes(self):
         """
         Return this buffer as a Python bytes object. Memory is copied.
@@ -212,7 +216,9 @@ cdef class Pickle5Writer:
                                    cpython.PyBUF_FULL_RO)
         buffer.set_length(view.len)
         buffer.set_ndim(view.ndim)
-        buffer.set_readonly(view.readonly)
+        # It should be 'view.readonly'. But for the sake of shared memory,
+        # we have to make it immutable.
+        buffer.set_readonly(1)
         buffer.set_itemsize(view.itemsize)
         if view.format:
             buffer.set_format(view.format)
diff --git a/python/ray/tests/test_basic.py b/python/ray/tests/test_basic.py
index 9af8eb798..19557a9ac 100644
--- a/python/ray/tests/test_basic.py
+++ b/python/ray/tests/test_basic.py
@@ -492,6 +492,15 @@ def test_reducer_override_no_reference_cycle(ray_start_regular):
     assert new_obj() is None
 
 
+def test_deserialized_from_buffer_immutable(ray_start_regular):
+    x = np.full((2, 2), 1.)
+    o = ray.put(x)
+    y = ray.get(o)
+    with pytest.raises(
+            ValueError, match="assignment destination is read-only"):
+        y[0, 0] = 9.
+
+
 def test_passing_arguments_by_value_out_of_the_box(ray_start_regular):
     @ray.remote
     def f(x):