-
-
Notifications
You must be signed in to change notification settings - Fork 763
Fix Issue 16824 - std.experimental.allocator.dispose leaks memory #4982
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
935d4ad
936a802
098e52b
23fcc1c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1600,6 +1600,132 @@ unittest //bugzilla 15721 | |
| Mallocator.instance.dispose(foo); | ||
| } | ||
|
|
||
| /** | ||
| Allocates a multidimensional array of elements of type T. | ||
|
|
||
| Params: | ||
| N = number of dimensions | ||
| T = element type of an element of the multidimensional arrat | ||
| alloc = the allocator used for getting memory | ||
| lengths = static array containing the size of each dimension | ||
|
|
||
| Returns: | ||
| An N-dimensional array with individual elements of type T. | ||
| */ | ||
| auto makeMultidimensionalArray(uint n, T, Allocator)(auto ref Allocator alloc, size_t[n] lengths) | ||
| { | ||
| static if (n == 1) | ||
| { | ||
| return makeArray!T(alloc, lengths[0]); | ||
| } | ||
| else | ||
| { | ||
| alias E = typeof(makeMultidimensionalArray!(n - 1, T)(alloc, lengths[1..$])); | ||
| auto ret = makeArray!E(alloc, lengths[0]); | ||
| foreach (ref e; ret) | ||
| e = makeMultidimensionalArray!(n - 1, T)(alloc, lengths[1..$]); | ||
| return ret; | ||
| } | ||
| } | ||
|
|
||
| /// | ||
| unittest | ||
| { | ||
| import std.experimental.allocator.mallocator : Mallocator; | ||
|
|
||
| size_t[3] dimArray = [2, 3, 6]; | ||
| auto mArray = Mallocator.instance.makeMultidimensionalArray!(dimArray.length, int)(dimArray); | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. guess it's nice to also deallocate here with |
||
| // deallocate when exiting scope | ||
| scope(exit) | ||
| { | ||
| Mallocator.instance.disposeMultidimensionalArray(mArray); | ||
| } | ||
|
|
||
| assert(mArray.length == 2); | ||
| foreach (lvl2Array; mArray) | ||
| { | ||
| assert(lvl2Array.length == 3); | ||
| foreach (lvl3Array; lvl2Array) | ||
| assert(lvl3Array.length == 6); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| Destroys and then deallocates a multidimensional array, assuming it was | ||
| created with makeMultidimensionalArray and the same allocator was used. | ||
|
|
||
| Params: | ||
| T = element type of an element of the multidimensional array | ||
| alloc = the allocator used for getting memory | ||
| array = the multidimensional array that is to be deallocated | ||
| */ | ||
| void disposeMultidimensionalArray(Allocator, T)(auto ref Allocator alloc, T[] array) | ||
| { | ||
| static if (isArray!T) | ||
| { | ||
| foreach (ref e; array) | ||
| disposeMultidimensionalArray(alloc, e); | ||
| } | ||
|
|
||
| dispose(alloc, array); | ||
| } | ||
|
|
||
| /// | ||
| unittest | ||
| { | ||
| struct TestAllocator | ||
| { | ||
| import std.experimental.allocator.common : platformAlignment; | ||
| import std.experimental.allocator.mallocator : Mallocator; | ||
|
|
||
| alias allocator = Mallocator.instance; | ||
|
|
||
| private static struct ByteRange | ||
| { | ||
| void* ptr; | ||
| size_t length; | ||
| } | ||
|
|
||
| private ByteRange[] _allocations; | ||
|
|
||
| enum uint alignment = platformAlignment; | ||
|
|
||
| void[] allocate(size_t numBytes) | ||
| { | ||
| auto ret = allocator.allocate(numBytes); | ||
| _allocations ~= ByteRange(ret.ptr, ret.length); | ||
| return ret; | ||
| } | ||
|
|
||
| bool deallocate(void[] bytes) | ||
| { | ||
| import std.algorithm.mutation : remove; | ||
| import std.algorithm.searching : canFind; | ||
|
|
||
| bool pred(ByteRange other) | ||
| { return other.ptr == bytes.ptr && other.length == bytes.length; } | ||
|
|
||
| assert(_allocations.canFind!pred); | ||
|
|
||
| _allocations = _allocations.remove!pred; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FYI: there is |
||
| return allocator.deallocate(bytes); | ||
| } | ||
|
|
||
| ~this() | ||
| { | ||
| assert(!_allocations.length); | ||
| } | ||
| } | ||
|
|
||
| TestAllocator allocator; | ||
|
|
||
| size_t[5] a = [2, 3, 6, 7, 2]; | ||
| auto mArray = allocator.makeMultidimensionalArray!(a.length, int)(a); | ||
|
|
||
| allocator.disposeMultidimensionalArray(mArray); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about doing the same test with something that has a destructor and thus verifying that the destructor is called for all elements? e.g. static instances = 0;
struct Foo
{
~this()
{
instances = 0;
}
// allocate
assert(instances == 100);
// dispose
assert(instances == 0); |
||
|
|
||
| /** | ||
|
|
||
| Returns a dynamically-typed $(D CAllocator) built around a given statically- | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't use capital names for values, just use
nThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought that this is common for statically defined arrays [1][2]
[1] https://dlang.org/library/std/experimental/ndslice/selection/iota_slice.html
[2] https://github.com/dlang/phobos/blob/master/std/experimental/ndslice/slice.d#L834
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx for the good examples, though no exception is justified there. We should probably slowly fix those as the code gets touched instead of introducing more instances.
cc @wilzbach: all values and functions are
likeThis, all types areLikeThis, aliases that may be either should generally belikeThis, modules and labels should belike_this.