Skip to content
This repository has been archived by the owner on Mar 9, 2023. It is now read-only.

Commit

Permalink
Don't explicitly release the memoryview in Grammar (#127)
Browse files Browse the repository at this point in the history
This should fix the issue in #126 without sacrificing speed.

The BufferError is caused by the Grammar calling release on its
memoryview when it's deleted. With the error-causing code, that doesn't
work because the Lattice has a reference to the memoryview, which is why
the BufferError is thrown.

The BufferError would represent a real problem if the Lattice was used
after the Grammar was deleted, but that can't happen normally, and would
throw all kinds of errors even with pre-Cython code.

It seems natural for a class to release a memoryview it created, so I
didn't think it was odd at first, but a memoryview is memory managed
like any other Python object, so there's no need to manually release it
unless you need to remove restrictions on the underlying data or
something. It's fine to just let gc take care of it. In this case,
manually releasing it causes issues because it's possible for the
Grammar to be deleted before the Lattice. (I don't entirely follow why
this only happens sometimes, but I think it has to do with the
BinaryDictionary explicitly deleting the Grammar.)

So now what happens is that the Grammar is created with a bytes object,
creates a memoryview with a ref to the bytes object, and the Lattice
creates a ref to the memoryview. Once the references to the Grammar and
Lattice are removed (regardless of order), gc will take care of them
appropriately.
  • Loading branch information
polm authored Jun 15, 2020
1 parent 620f7c3 commit 1fa7774
Showing 1 changed file with 0 additions and 3 deletions.
3 changes: 0 additions & 3 deletions sudachipy/dictionarylib/grammar.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,6 @@ def __init__(self, bytes_, offset):
self._matrix_view = self._matrix_view.cast('h', shape=[left_id_size, right_id_size])
bytes_.seek(original_offset)

def __del__(self):
self._matrix_view.release()

def get_storage_size(self):
return self.storage_size

Expand Down

0 comments on commit 1fa7774

Please sign in to comment.