Hi,

the following coding is an extract from the DGUI library.
On my system (Win 8) I receive InvalidMemoryOperationError@.
I got the information that this Error doesn't not occur on
a WinXP system.

I think there is definetely a bug in the following coding.

The Grid class has a Collection of Rows. Each Row has a Collection
of Cols.
The class Row has a destructor which has an empty foreach over
the Collection of Cols. If I comment this empty foreach no
error is thrown. I assume the class Collection accesses a Col
object which is already destructed?

Does DMD behave correctly (InvalidMemoryOperationError) and
can where in the Collection class exactly the invalid object is
accessed? I tried to find the invalid coding line, but without success.

class Collection(T)
{
	private T[] _t;

	int add(T t)
	{
		this._t ~= t;
		return this._t.length - 1;
	}

	void clear()
	{
		this._t.length = 0;
	}

	T[] get()
	{
		return this._t;
	}

	@property int length()
	{
		return this._t.length;
	}

	T opIndex(int i) nothrow
	{
		if(i >= 0 && i < this._t.length)
		{
			return this._t[i];
		}
		assert(false, "Index out of range");
	}

	int opApply(int delegate(ref T) dg)
	{
		int res = 0;

		if(this._t.length)
		{
			for(int i = 0; i < this._t.length; i++)
			{
				res = dg(this._t[i]);

				if(res)
				{
					break;
				}
			}
		}
		return res;
	}
}

class Col {}

class Row
{
	private Collection!(Col) _columns;
	
	this()
	{
		this._columns = new Collection!(Col)();
	}
	
	~this()
	{
		foreach(cp; this._columns)
		{	
		}
	}
	
	
	Col addColumn()
	{
		Col cp = new Col();
		this._columns.add(cp);
		return cp;
	}
}

class Grid
{
	private Collection!(Row) _rows;
	
	this()
	{
		this._rows = new Collection!(Row)();
	}
	
	@property public Row[] rows()
	{
		return this._rows.get();
	}
	
	Row addRow()
	{	
		Row rp = new Row();
		this._rows.add(rp);
		return rp;
	}
	
	void clear()
	{
		_rows.clear();
	}
}

void main()
{
	auto grid = new Grid();

	for(int i=0; i< 100000; i++)
	{
		grid.clear();

		with(grid.addRow())
		{
			addColumn();
			addColumn();
			addColumn();
		}
	}
}

On Mon, 17 Nov 2014 15:41:25 +0000
Andre via Digitalmars-d-learn <digitalmars-d-learn@puremagic.com> wrote:

> 	~this()
> 	{
> 		foreach(cp; this._columns)
> 		{
> 		}
> 	}
don't do that in destructors. `_columns` field can be already collected by GC.

Thanks a lot.
I will forward this recommendation (DGUI BitBucket).

Kind regards
André

On Monday, 17 November 2014 at 16:40:18 UTC, ketmar via
Digitalmars-d-learn wrote:
> On Mon, 17 Nov 2014 15:41:25 +0000
> Andre via Digitalmars-d-learn <digitalmars-d-learn@puremagic.com> wrote:
>
>> 	~this()
>> 	{
>> 		foreach(cp; this._columns)
>> 		{	
>> 		}
>> 	}
> don't do that in destructors. `_columns` field can be already collected
> by GC.

On Monday, 17 November 2014 at 16:40:18 UTC, ketmar via Digitalmars-d-learn wrote:
> On Mon, 17 Nov 2014 15:41:25 +0000
> Andre via Digitalmars-d-learn <digitalmars-d-learn@puremagic.com> wrote:
>
>> 	~this()
>> 	{
>> 		foreach(cp; this._columns)
>> 		{	
>> 		}
>> 	}
> don't do that in destructors. `_columns` field can be already collected
> by GC.

Last I checked, the GC finalizes all dead objects before freeing them. InvalidMemoryOperation indicates that an allocation occurred, where is it?

I can't reproduce the problem with D from git HEAD.

On Monday, 17 November 2014 at 22:19:04 UTC, Vladimir Panteleev wrote:
> On Monday, 17 November 2014 at 16:40:18 UTC, ketmar via Digitalmars-d-learn wrote:
>> On Mon, 17 Nov 2014 15:41:25 +0000
>> Andre via Digitalmars-d-learn <digitalmars-d-learn@puremagic.com> wrote:
>>
>>> 	~this()
>>> 	{
>>> 		foreach(cp; this._columns)
>>> 		{	
>>> 		}
>>> 	}
>> don't do that in destructors. `_columns` field can be already collected
>> by GC.
>
> Last I checked, the GC finalizes all dead objects before freeing them. InvalidMemoryOperation indicates that an allocation occurred, where is it?

Found it. opApply is virtual, so the vtable is needed to call it. The pointer is cleared during destruction, thus an access violation occurs, which is attempted to be translated into a D exception. However, when D attempts to allocate it, an InvalidMemoryOperation occurs.

On 11/17/14 5:19 PM, Vladimir Panteleev wrote:
> On Monday, 17 November 2014 at 16:40:18 UTC, ketmar via
> Digitalmars-d-learn wrote:
>> On Mon, 17 Nov 2014 15:41:25 +0000
>> Andre via Digitalmars-d-learn <digitalmars-d-learn@puremagic.com> wrote:
>>
>>>     ~this()
>>>     {
>>>         foreach(cp; this._columns)
>>>         {
>>>         }
>>>     }
>> don't do that in destructors. `_columns` field can be already collected
>> by GC.
>
> Last I checked, the GC finalizes all dead objects before freeing them.

The GC is not guaranteed to finalize them all before freeing them all.

I would not count on that property -- even if it's currently true. Expect that any GC-allocated memory in your dtor is invalid except for the memory of the object itself.

-Steve

On Monday, 17 November 2014 at 22:40:36 UTC, Steven Schveighoffer wrote:
> On 11/17/14 5:19 PM, Vladimir Panteleev wrote:
>> On Monday, 17 November 2014 at 16:40:18 UTC, ketmar via
>> Digitalmars-d-learn wrote:
>>> On Mon, 17 Nov 2014 15:41:25 +0000
>>> Andre via Digitalmars-d-learn <digitalmars-d-learn@puremagic.com> wrote:
>>>
>>>>    ~this()
>>>>    {
>>>>        foreach(cp; this._columns)
>>>>        {
>>>>        }
>>>>    }
>>> don't do that in destructors. `_columns` field can be already collected
>>> by GC.
>>
>> Last I checked, the GC finalizes all dead objects before freeing them.
>
> The GC is not guaranteed to finalize them all before freeing them all.
>
> I would not count on that property -- even if it's currently true. Expect that any GC-allocated memory in your dtor is invalid except for the memory of the object itself.

That's disappointing because it makes destructors considerably less useful. I think that at this point, this will probably become a guarantee for compatibility with existing code.

On 11/17/14 6:38 PM, Vladimir Panteleev wrote:
> On Monday, 17 November 2014 at 22:40:36 UTC, Steven Schveighoffer wrote:
>> On 11/17/14 5:19 PM, Vladimir Panteleev wrote:
>>> On Monday, 17 November 2014 at 16:40:18 UTC, ketmar via
>>> Digitalmars-d-learn wrote:
>>>> On Mon, 17 Nov 2014 15:41:25 +0000
>>>> Andre via Digitalmars-d-learn <digitalmars-d-learn@puremagic.com>
>>>> wrote:
>>>>
>>>>>    ~this()
>>>>>    {
>>>>>        foreach(cp; this._columns)
>>>>>        {
>>>>>        }
>>>>>    }
>>>> don't do that in destructors. `_columns` field can be already collected
>>>> by GC.
>>>
>>> Last I checked, the GC finalizes all dead objects before freeing them.
>>
>> The GC is not guaranteed to finalize them all before freeing them all.
>>
>> I would not count on that property -- even if it's currently true.
>> Expect that any GC-allocated memory in your dtor is invalid except for
>> the memory of the object itself.
>
> That's disappointing because it makes destructors considerably less
> useful. I think that at this point, this will probably become a
> guarantee for compatibility with existing code.

destructors are *strictly* for freeing non-GC resources. That has been in the spec from Day 1 (well, at least the earliest I can remember), and it's the same for other GC-based systems as well, such as Java.

I don't want to say that making this requirement would hinder performance, but I'd hate to lock us into the current GC model, as it pretty much sucks performance-wise. I'd rather keep as many options open as possible, to allow GC tinkering for performance.

-Steve