Skip to content

Commit d6db3ad

Browse files
msimacekmsimacek
committed
[GR-26945] Implement buffer locking for arrays and bytearrays
PullRequest: graalpython/1412
2 parents 0118930 + f5b95db commit d6db3ad

File tree

23 files changed

+668
-443
lines changed

23 files changed

+668
-443
lines changed

‎graalpython/com.oracle.graal.python.cext/src/bytearrayobject.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -54,5 +54,10 @@ int bytearray_getbuffer(PyByteArrayObject *obj, Py_buffer *view, int flags) {
5454
ptr = (void *) PyByteArray_AS_STRING(obj);
5555
/* cannot fail if view != NULL and readonly == 0 */
5656
(void)PyBuffer_FillInfo(view, (PyObject*)obj, ptr, Py_SIZE(obj), 0, flags);
57+
obj->ob_exports++;
5758
return 0;
5859
}
60+
61+
void bytearray_releasebuffer(PyByteArrayObject *obj, Py_buffer *view) {
62+
obj->ob_exports--;
63+
}

‎graalpython/com.oracle.graal.python.cext/src/capi.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ static void initialize_globals() {
247247

248248
static void initialize_bufferprocs() {
249249
polyglot_invoke(PY_TRUFFLE_CEXT, "PyTruffle_SetBufferProcs", native_to_java((PyObject*)&PyBytes_Type), (getbufferproc)bytes_buffer_getbuffer, (releasebufferproc)NULL);
250-
polyglot_invoke(PY_TRUFFLE_CEXT, "PyTruffle_SetBufferProcs", native_to_java((PyObject*)&PyByteArray_Type), (getbufferproc)bytearray_getbuffer, (releasebufferproc)NULL);
250+
polyglot_invoke(PY_TRUFFLE_CEXT, "PyTruffle_SetBufferProcs", native_to_java((PyObject*)&PyByteArray_Type), (getbufferproc)bytearray_getbuffer, (releasebufferproc)bytearray_releasebuffer);
251251
polyglot_invoke(PY_TRUFFLE_CEXT, "PyTruffle_SetBufferProcs", native_to_java((PyObject*)&PyBuffer_Type), (getbufferproc)bufferdecorator_getbuffer, (releasebufferproc)NULL);
252252
polyglot_invoke(PY_TRUFFLE_CEXT, "PyTruffle_SetBufferProcs", native_to_java((PyObject*)&PyMemoryView_Type), (getbufferproc)memoryview_getbuffer, (releasebufferproc)memoryview_releasebuffer);
253253
}
@@ -511,6 +511,12 @@ PyObject* PyTruffle_Object_New(PyTypeObject* cls, PyTypeObject* dominatingNative
511511
} \
512512
return polyglot_from_##__polyglot_type__##_array(carr, len); \
513513
} \
514+
void* PyTruffle_##__jtype__##ArrayRealloc(const void* array, int64_t len) { \
515+
int64_t size = len + 1; \
516+
__ctype__* carr = (__ctype__*) realloc(array, size * sizeof(__ctype__)); \
517+
carr[len] = (__ctype__)0; \
518+
return polyglot_from_##__polyglot_type__##_array(carr, len); \
519+
}
514520

515521
PRIMITIVE_ARRAY_TO_NATIVE(Byte, int8_t, i8, polyglot_as_i8);
516522
PRIMITIVE_ARRAY_TO_NATIVE(Int, int32_t, i32, polyglot_as_i32);

‎graalpython/com.oracle.graal.python.cext/src/capi.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,7 @@ extern PyObject* wrapped_null;
385385
/* BYTES, BYTEARRAY */
386386
int bytes_buffer_getbuffer(PyBytesObject *self, Py_buffer *view, int flags);
387387
int bytearray_getbuffer(PyByteArrayObject *obj, Py_buffer *view, int flags);
388+
void bytearray_releasebuffer(PyByteArrayObject *obj, Py_buffer *view);
388389

389390
/* Like 'memcpy' but can read/write from/to managed objects. */
390391
int bytes_copy2mem(char* target, char* source, size_t nbytes);

‎graalpython/com.oracle.graal.python.test/src/tests/cpyext/test_bytes.py

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -337,3 +337,46 @@ def compile_module(self, name):
337337
callfunction="resize_bytes",
338338
cmpfunc=unhandled_error_compare
339339
)
340+
341+
test_bytearray_buffer = CPyExtFunction(
342+
lambda args: args[1],
343+
lambda: (
344+
(bytearray(b"hello_world"), b"Hello_worlds"),
345+
),
346+
code="""
347+
PyObject* test_buffer(PyObject* bytes, PyObject* expected) {
348+
Py_buffer buffer;
349+
PyObject* ret;
350+
if (PyObject_GetBuffer(bytes, &buffer, PyBUF_SIMPLE | PyBUF_WRITABLE) != 0)
351+
return NULL;
352+
*(char*)buffer.buf = 'H';
353+
Py_ssize_t len = PyObject_Size(bytes);
354+
if (len == -1)
355+
goto error_release;
356+
ret = PyObject_CallMethod(bytes, "insert", "ni", len, 'x');
357+
if (ret != NULL) {
358+
Py_DECREF(ret);
359+
PyErr_SetString(PyExc_AssertionError, "insert didn't raise BufferError");
360+
goto error_release;
361+
}
362+
if (!PyErr_ExceptionMatches(PyExc_BufferError))
363+
goto error_release;
364+
PyErr_Clear();
365+
PyBuffer_Release(&buffer);
366+
ret = PyObject_CallMethod(bytes, "insert", "ni", len, 's');
367+
if (ret == NULL)
368+
return NULL;
369+
Py_DECREF(ret);
370+
Py_INCREF(bytes);
371+
return bytes;
372+
error_release:
373+
PyBuffer_Release(&buffer);
374+
return NULL;
375+
}
376+
""",
377+
resultspec="O",
378+
argspec="OO",
379+
arguments=["PyObject* bytes", "PyObject* expected"],
380+
callfunction="test_buffer",
381+
cmpfunc=unhandled_error_compare
382+
)

‎graalpython/com.oracle.graal.python.test/src/tests/test_memoryview.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
#
44
# Licensed under the PYTHON SOFTWARE FOUNDATION LICENSE VERSION 2
55

6+
import sys
7+
import pytest
68

79
def test_subscript():
810
v = memoryview(b'abcefg')
@@ -162,3 +164,21 @@ def test_pack():
162164
assert b == b'\x01'
163165
memoryview(b).cast('?')[0] = False
164166
assert b == b'\x00'
167+
168+
def test_read_after_resize():
169+
if sys.implementation.name != "graalpython":
170+
return
171+
# CPython prevents resizing of acquired buffers at all to avoid a segfault
172+
# We don't want to impose locking on managed objects because we cannot automatically
173+
# release the lock by reference counting. Check that we don't hard crash when
174+
# does an out-of-bound read on a resized buffer
175+
b = bytearray(b'12341251452134523463456435643')
176+
m = memoryview(b)
177+
assert m[1] == ord('2')
178+
b.clear()
179+
with pytest.raises(IndexError):
180+
print(m[1])
181+
with pytest.raises(IndexError):
182+
m[1] = 3
183+
with pytest.raises(IndexError):
184+
print(m.tobytes())

‎graalpython/com.oracle.graal.python.test/src/tests/unittest_tags/test_bytes.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@
126126
*graalpython.lib-python.3.test.test_bytes.ByteArrayTest.test_repeat_1char
127127
*graalpython.lib-python.3.test.test_bytes.ByteArrayTest.test_replace
128128
*graalpython.lib-python.3.test.test_bytes.ByteArrayTest.test_replace_int_error
129+
*graalpython.lib-python.3.test.test_bytes.ByteArrayTest.test_resize_forbidden
129130
*graalpython.lib-python.3.test.test_bytes.ByteArrayTest.test_reverse
130131
*graalpython.lib-python.3.test.test_bytes.ByteArrayTest.test_reversed
131132
*graalpython.lib-python.3.test.test_bytes.ByteArrayTest.test_rfind

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/BuiltinConstructors.java

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,6 @@
156156
import com.oracle.graal.python.builtins.objects.iterator.PZip;
157157
import com.oracle.graal.python.builtins.objects.list.PList;
158158
import com.oracle.graal.python.builtins.objects.map.PMap;
159-
import com.oracle.graal.python.builtins.objects.memoryview.ManagedBuffer;
160159
import com.oracle.graal.python.builtins.objects.memoryview.MemoryViewNodes;
161160
import com.oracle.graal.python.builtins.objects.memoryview.PBuffer;
162161
import com.oracle.graal.python.builtins.objects.memoryview.PMemoryView;
@@ -3395,32 +3394,28 @@ public final PMemoryView execute(VirtualFrame frame, Object object) {
33953394

33963395
@Specialization
33973396
PMemoryView fromBytes(@SuppressWarnings("unused") Object cls, PBytes object,
3398-
@Shared("getQueue") @Cached MemoryViewNodes.GetBufferReferences getQueue,
33993397
@Cached SequenceNodes.GetSequenceStorageNode getSequenceStorageNode,
34003398
@Cached SequenceStorageNodes.LenNode lenNode) {
34013399
SequenceStorage storage = getSequenceStorageNode.execute(object);
3402-
return fromManaged(object, 1, lenNode.execute(storage), true, "B", false, getQueue.execute());
3400+
return factory().createMemoryViewForManagedObject(object, 1, lenNode.execute(storage), true, "B");
34033401
}
34043402

34053403
@Specialization
34063404
PMemoryView fromByteArray(@SuppressWarnings("unused") Object cls, PByteArray object,
3407-
@Shared("getQueue") @Cached MemoryViewNodes.GetBufferReferences getQueue,
34083405
@Cached SequenceNodes.GetSequenceStorageNode getSequenceStorageNode,
34093406
@Cached SequenceStorageNodes.LenNode lenNode) {
34103407
SequenceStorage storage = getSequenceStorageNode.execute(object);
3411-
return fromManaged(object, 1, lenNode.execute(storage), false, "B", true, getQueue.execute());
3408+
return factory().createMemoryViewForManagedObject(object, 1, lenNode.execute(storage), false, "B");
34123409
}
34133410

34143411
@Specialization
3415-
PMemoryView fromArray(@SuppressWarnings("unused") Object cls, PArray object,
3416-
@Shared("getQueue") @Cached MemoryViewNodes.GetBufferReferences getQueue) {
3417-
int itemsize = object.getFormat().bytesize;
3418-
return fromManaged(object, itemsize, object.getLength(), false, object.getFormatStr(), true, getQueue.execute());
3412+
PMemoryView fromArray(@SuppressWarnings("unused") Object cls, PArray object) {
3413+
return factory().createMemoryViewForManagedObject(object, object.getFormat().bytesize, object.getLength(), false, object.getFormatStr());
34193414
}
34203415

34213416
@Specialization
34223417
PMemoryView fromMemoryView(@SuppressWarnings("unused") Object cls, PMemoryView object,
3423-
@Shared("getQueue") @Cached MemoryViewNodes.GetBufferReferences getQueue) {
3418+
@Cached MemoryViewNodes.GetBufferReferences getQueue) {
34243419
object.checkReleased(this);
34253420
return factory().createMemoryView(getQueue.execute(), object.getManagedBuffer(), object.getOwner(), object.getLength(),
34263421
object.isReadOnly(), object.getItemSize(), object.getFormat(), object.getFormatString(), object.getDimensions(),
@@ -3450,18 +3445,6 @@ PMemoryView error(@SuppressWarnings("unused") Object cls, Object object) {
34503445
throw raise(TypeError, ErrorMessages.MEMORYVIEW_A_BYTES_LIKE_OBJECT_REQUIRED_NOT_P, object);
34513446
}
34523447

3453-
private PMemoryView fromManaged(Object object, int itemsize, int length, boolean readonly, String format, boolean needsRelease,
3454-
MemoryViewNodes.BufferReferences refQueue) {
3455-
ManagedBuffer managedBuffer = null;
3456-
if (needsRelease) {
3457-
// TODO We should lock the underlying storage for resizing
3458-
managedBuffer = ManagedBuffer.createForManaged(object);
3459-
}
3460-
return factory().createMemoryView(refQueue, managedBuffer, object, length * itemsize, readonly, itemsize, format, 1,
3461-
null, 0, new int[]{length}, new int[]{itemsize}, null,
3462-
PMemoryView.FLAG_C | PMemoryView.FLAG_FORTRAN);
3463-
}
3464-
34653448
public static MemoryViewNode create() {
34663449
return BuiltinConstructorsFactory.MemoryViewNodeFactory.create(null);
34673450
}

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/modules/PythonCextBuiltins.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1625,7 +1625,7 @@ Object wrap(VirtualFrame frame, Object bufferStructPointer, Object ownerObj, Obj
16251625
int flags = initFlagsNode.execute(ndim, itemsize, shape, strides, suboffsets);
16261626
ManagedBuffer managedBuffer = null;
16271627
if (!lib.isNull(bufferStructPointer)) {
1628-
managedBuffer = ManagedBuffer.createForNative(bufferStructPointer);
1628+
managedBuffer = new ManagedBuffer(bufferStructPointer);
16291629
}
16301630
PMemoryView memoryview = factory().createMemoryView(getQueue.execute(), managedBuffer, owner, len, readonly, itemsize,
16311631
BufferFormat.forMemoryView(format),

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/array/ArrayBuiltins.java

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,9 @@ abstract static class IMulNode extends PythonBinaryClinicBuiltinNode {
201201
Object concat(PArray self, int value) {
202202
try {
203203
int newLength = Math.max(PythonUtils.multiplyExact(self.getLength(), value), 0);
204+
if (newLength != self.getLength()) {
205+
self.checkCanResize(this);
206+
}
204207
int itemsize = self.getFormat().bytesize;
205208
int segmentLength = self.getLength() * itemsize;
206209
self.resize(newLength);
@@ -533,6 +536,7 @@ Object setitem(PArray self, PSlice slice, PArray other,
533536
}
534537
if (simpleStepProfile.profile(step == 1)) {
535538
if (differentLengthProfile.profile(sliceLength != needed)) {
539+
self.checkCanResize(this);
536540
if (growProfile.profile(sliceLength < needed)) {
537541
if (stop < start) {
538542
stop = start;
@@ -579,19 +583,21 @@ abstract static class DelItemNode extends PythonBinaryBuiltinNode {
579583
public abstract Object executeSlice(PArray self, PSlice slice);
580584

581585
@Specialization(guards = "!isPSlice(idx)", limit = "3")
582-
static Object delitem(PArray self, Object idx,
586+
Object delitem(PArray self, Object idx,
583587
@CachedLibrary("idx") PythonObjectLibrary lib,
584588
@Cached("forArrayAssign()") NormalizeIndexNode normalizeIndexNode) {
589+
self.checkCanResize(this);
585590
int index = normalizeIndexNode.execute(lib.asIndex(idx), self.getLength());
586591
self.delSlice(index, 1);
587592
return PNone.NONE;
588593
}
589594

590595
@Specialization
591-
static Object delitem(PArray self, PSlice slice,
596+
Object delitem(PArray self, PSlice slice,
592597
@Cached ConditionProfile simpleStepProfile,
593598
@Cached SliceLiteralNode.SliceUnpack sliceUnpack,
594599
@Cached SliceLiteralNode.AdjustIndices adjustIndices) {
600+
self.checkCanResize(this);
595601
int length = self.getLength();
596602
PSlice.SliceInfo sliceInfo = adjustIndices.execute(length, sliceUnpack.execute(slice));
597603
int start = sliceInfo.start;
@@ -713,6 +719,7 @@ Object append(VirtualFrame frame, PArray self, Object value,
713719
try {
714720
int index = self.getLength();
715721
int newLength = PythonUtils.addExact(index, 1);
722+
self.checkCanResize(this);
716723
self.resize(newLength);
717724
putValueNode.execute(frame, self, index, value);
718725
return PNone.NONE;
@@ -730,6 +737,9 @@ abstract static class ExtendNode extends PythonBinaryBuiltinNode {
730737
Object extend(PArray self, PArray value) {
731738
try {
732739
int newLength = PythonUtils.addExact(self.getLength(), value.getLength());
740+
if (newLength != self.getLength()) {
741+
self.checkCanResize(this);
742+
}
733743
int itemsize = self.getFormat().bytesize;
734744
self.resizeStorage(newLength);
735745
PythonUtils.arraycopy(value.getBuffer(), 0, self.getBuffer(), self.getLength() * itemsize, value.getLength() * itemsize);
@@ -750,7 +760,11 @@ Object extend(VirtualFrame frame, PArray self, PSequence value,
750760
SequenceStorage storage = getSequenceStorageNode.execute(value);
751761
int storageLength = lenNode.execute(storage);
752762
try {
753-
self.resizeStorage(PythonUtils.addExact(self.getLength(), storageLength));
763+
int newLength = PythonUtils.addExact(self.getLength(), storageLength);
764+
if (newLength != self.getLength()) {
765+
self.checkCanResize(this);
766+
}
767+
self.resizeStorage(newLength);
754768
} catch (OverflowException e) {
755769
CompilerDirectives.transferToInterpreterAndInvalidate();
756770
throw raise(MemoryError);
@@ -786,6 +800,7 @@ Object extend(VirtualFrame frame, PArray self, Object value,
786800
// in CPython
787801
try {
788802
length = PythonUtils.addExact(length, 1);
803+
self.checkCanResize(this);
789804
self.resizeStorage(length);
790805
} catch (OverflowException e) {
791806
CompilerDirectives.transferToInterpreterAndInvalidate();
@@ -825,6 +840,7 @@ Object insert(VirtualFrame frame, PArray self, int inputIndex, Object value,
825840
// Need to check the validity of the value before moving the memory around to ensure the
826841
// operation can fail atomically
827842
checkValueNode.execute(frame, self, value);
843+
self.checkCanResize(this);
828844
try {
829845
self.shift(index, 1);
830846
} catch (OverflowException e) {
@@ -851,6 +867,7 @@ Object remove(VirtualFrame frame, PArray self, Object value,
851867
for (int i = 0; i < self.getLength(); i++) {
852868
Object item = getValueNode.execute(self, i);
853869
if (lib.equalsWithFrame(item, value, lib, frame)) {
870+
self.checkCanResize(this);
854871
self.delSlice(i, 1);
855872
return PNone.NONE;
856873
}
@@ -872,6 +889,7 @@ Object pop(PArray self, int inputIndex,
872889
}
873890
int index = normalizeIndexNode.execute(inputIndex, self.getLength());
874891
Object value = getValueNode.execute(self, index);
892+
self.checkCanResize(this);
875893
self.delSlice(index, 1);
876894
return value;
877895
}
@@ -898,6 +916,7 @@ Object frombytes(PArray self, Object buffer,
898916
}
899917
int newLength = PythonUtils.addExact(oldSize, bufferLength / itemsize);
900918
byte[] bufferBytes = lib.getBufferBytes(buffer);
919+
self.checkCanResize(this);
901920
self.resize(newLength);
902921
PythonUtils.arraycopy(bufferBytes, 0, self.getBuffer(), oldSize * itemsize, bufferLength);
903922
} catch (UnsupportedMessageException e) {
@@ -965,6 +984,7 @@ Object fromlist(VirtualFrame frame, PArray self, PList list,
965984
SequenceStorage storage = getSequenceStorageNode.execute(list);
966985
int length = lenNode.execute(storage);
967986
int newLength = PythonUtils.addExact(self.getLength(), length);
987+
self.checkCanResize(this);
968988
self.resizeStorage(newLength);
969989
for (int i = 0; i < length; i++) {
970990
putValueNode.execute(frame, self, self.getLength() + i, getItemScalarNode.execute(storage, i));
@@ -1017,6 +1037,7 @@ Object fromunicode(VirtualFrame frame, PArray self, String str,
10171037
try {
10181038
int length = PString.codePointCount(str, 0, str.length());
10191039
int newLength = PythonUtils.addExact(self.getLength(), length);
1040+
self.checkCanResize(this);
10201041
self.resizeStorage(newLength);
10211042
for (int codePointIndex = 0, charIndex = 0; codePointIndex < length; codePointIndex++) {
10221043
int charCount = PString.charCount(PString.codePointAt(str, charIndex));

‎graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/array/PArray.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,15 @@
2525
*/
2626
package com.oracle.graal.python.builtins.objects.array;
2727

28+
import static com.oracle.graal.python.builtins.PythonBuiltinClassType.BufferError;
29+
2830
import java.nio.ByteOrder;
2931
import java.util.Arrays;
3032

3133
import com.oracle.graal.python.builtins.objects.object.PythonBuiltinObject;
3234
import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
35+
import com.oracle.graal.python.nodes.ErrorMessages;
36+
import com.oracle.graal.python.nodes.function.PythonBuiltinBaseNode;
3337
import com.oracle.graal.python.util.BufferFormat;
3438
import com.oracle.graal.python.util.OverflowException;
3539
import com.oracle.graal.python.util.PythonUtils;
@@ -46,6 +50,7 @@ public final class PArray extends PythonBuiltinObject {
4650
private String formatStr;
4751
private int length;
4852
private byte[] buffer;
53+
private volatile int exports;
4954

5055
public PArray(Object clazz, Shape instanceShape, String formatStr, BufferFormat format) {
5156
super(clazz, instanceShape);
@@ -84,6 +89,20 @@ public void setLength(int length) {
8489
this.length = length;
8590
}
8691

92+
public int getExports() {
93+
return exports;
94+
}
95+
96+
public void setExports(int exports) {
97+
this.exports = exports;
98+
}
99+
100+
public void checkCanResize(PythonBuiltinBaseNode node) {
101+
if (exports != 0) {
102+
throw node.raise(BufferError, ErrorMessages.EXPORTS_CANNOT_RESIZE);
103+
}
104+
}
105+
87106
private int computeNewSize(int newLength, int itemsize) throws OverflowException {
88107
int newSize = computeNewSizeNoOverflowCheck(newLength, itemsize);
89108
if (newSize / itemsize < newLength) {

0 commit comments

Comments
 (0)