www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - InvalidMemoryOperationError (0)

reply "Andre" <andre s-e-a-p.de> writes:
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();
		}
	}
}
Nov 17 2014
parent reply ketmar via Digitalmars-d-learn <digitalmars-d-learn puremagic.com> writes:
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)
 		{=09
 		}
 	}


don't do that in destructors. `_columns` field can be already collected
by GC.
Nov 17 2014
next sibling parent =?UTF-8?B?IkFuZHLDqSI=?= <andre s-e-a-p.de> writes:
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.
Nov 17 2014
prev sibling parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
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.
Nov 17 2014
next sibling parent "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
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.
Nov 17 2014
prev sibling parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
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
Nov 17 2014
parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
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.
Nov 17 2014
parent Steven Schveighoffer <schveiguy yahoo.com> writes:
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
Nov 17 2014