Feature idea: allow exposing read-only buffers

[bmerry]

Issue description

This is something that occurred to me when reading the Python docs on memoryview, but not something I have any immediate need for - which is why I've labelled it a "feature idea" rather than a "feature request".

At present, buffer_info doesn't have any indication of whether the memory should be treated as read-only or read-write. There may be a use case for a class to expose a buffer while not allowing writes to it. It looks like it shouldn't be too hard to add a flag to buffer_info and then add check in pybind11_buffer to raise a BufferError if the request has PyBUF_WRITABLE is set but the exposed buffer is read-only.

[bmerry]

Just spotted that this might be duplicated by #481, although it might not be quite the same thing.

[aldanor]

Yes this makes sense, should be a simple flag addition; in fact I had it implemented in one of the branches at some point but it wasn't merged in.

I believe numpy should respect this flag as well (and set the writeable flag accordingly), but this better be double checked.

[lramati]

Hi, I am trying to bind a project to Python using this library, and have just run in to this issue. I have an object with the function const void* get_data() that points to a couple megabytes of data. Right now I have to copy this into a std::vector<uint8_t> and send that to python, but this is very expensive. I was hoping to use py::buffer to get around this expensive copy, but can't because my data is read-only.

Can I have an explanation of how to modify pybind11 locally to expose this functionality, or an estimate of when this code might make it into the project?

Thanks

[complyue]

My +1 for I'm using pybind to expose mmap-ed array data to arm massive data on shared storage to multiple compute nodes, certainly majority of the mmaps are readonly.

If I don't touch pybind internal/detail like this:

  vector<array> bas; // bundle arrays
// ...
      // construct bundle array inplace
      bas.emplace_back(
          dt, // dtype
          // the explicit initializer_list construction is a necessary hint to compile,
          // plain {} won't work, even with explicit, element-wise type cast
          std::initializer_list<ssize_t>{scnt, slen}, // shape
          static_cast<void *>(mp + offset), // ptr
          mm // base
      );
      if (readonly_a) {
        // no public pybind api to manipulate array flags so far, have to use internal details until
        // https://github.com/aldanor/pybind11/pull/2 finds its way into release
        py::detail::array_proxy(bas.back().ptr())->flags &= ~py::detail::npy_api::NPY_ARRAY_WRITEABLE_;
      }

i.e. leave the array flags containing writeable, then accident write to such arrays will crash the process right away, rather than leave any hint for the problem like:

Traceback (most recent call last):
  File ".../interactiveshell.py", line 2862, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-6-5b2103cfff02>", line 1, in <module>
    a[2]=1
ValueError: assignment destination is read-only

[SimonRodrigue43]

Sebastian, what is the state of your modifications? Is it completed, tested and working? Could it be put in PR for merging?
Thanks.

[skoslowski]

There is a PR already: #1466
Concerning the state: This was an attempt to upstream some of the patches I have on a private fork. I never got any feedback on whether this would be considered for a merge. So, I eventually stopped updating the PR. If there is interest now, I'd be happy to continue.

[SimonRodrigue43]

This feature is very important in our situation because in some cases, buffer memory must stay immutable and a write could result in a crash. I also notice that we're not alone having that need by reading other comments.
@wjakob, is there some concerns with PR #1466 that could prevent it to be merged at some point?

[wjakob]

This PR is now merged, closing the ticket.