www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Do AAs allocate on removal?

reply Gerrit Wichert <gwichert yahoo.com> writes:
Hello,

I'm just bitten by some occasional finalize exceptions in a wxd program 
I'm writing on a OpenSuse 64 bit linux host with dmd 2.64.2.

I was able to track it down to a bug in wxd, where the wxObject 
destructor tried to remove the object from a static map.

some code from wxObject:

         ~this()
         {
             Dispose();
         }

         public /+virtual+/ void Dispose()
         {
             if (wxobj != IntPtr.init)
             {
             //    bool still_there = RemoveObject(wxobj);

             //    lock (typeof (wxObject)) {
                     if (memOwn /*&& still_there*/)
                     {
                         dtor();
                     }
             //    }

                 RemoveObject(wxobj);
                 wxobj = IntPtr.init;
             //    memOwn = false;
             }
             //GC.SuppressFinalize(this);
         }

         // Removes a registered object.
         // returns true if the object is found in the
         // Hashtable and is removed (for Dispose)

         public static bool RemoveObject(IntPtr ptr)
         {
             bool retval = false;

             if (ptr != IntPtr.init)
             {
                 if(ptr in objects) {
                   //gw 20131207 objects.remove(ptr); <-- here the 
application goes boom, so i replaced it with the following line
                   objects[ptr] = null;
                   retval = true;
                 }
             }

             return retval;
         }

         // Hashtable to associate C++ objects with D references
         private static wxObject[IntPtr] objects;



As you can see, the problem is that wxObject destructor tried to remove 
an entry from a static AA. Replacing the removal with just nulling the 
value solves the problem. But now I have to implement a cyclic cleanup 
routine wich removes the nulls from the AA. I really don't want to do that.

I suspect that the AA reorganized (with allocation) when an element is 
removed. I think it is a good idee if this can be delayed until the next 
insert, so that remove can be used in finalizers.

I also don't have a clue why the object gets collected when it is 
referenced in the static map.


Gerrit Wichert
Dec 07 2013
parent reply "thedeemon" <dlang thedeemon.com> writes:
On Saturday, 7 December 2013 at 15:58:08 UTC, Gerrit Wichert 
wrote:

 I suspect that the AA reorganized (with allocation) when an 
 element is removed. I think it is a good idee if this can be 
 delayed until the next insert, so that remove can be used in 
 finalizers.


A quick glance into the source shows that removing an entry does 
not allocate, but calls GC.free(). Inserting an entry does 
allocate. Rehashing happens when entries are added, not when they 
are removed.

Check your destructors again, make sure they are not called twice.
Dec 07 2013
next sibling parent reply Gerrit Wichert <gwichert yahoo.com> writes:
Am 07.12.2013 19:07, schrieb thedeemon:

 A quick glance into the source shows that removing an entry does not 
 allocate, but calls GC.free(). Inserting an entry does allocate. 
 Rehashing happens when entries are added, not when they are removed.

 Check your destructors again, make sure they are not called twice.


Thanks for your quick answer. So the call to GC.free() breaks it. Seems 
easy to fix, just don't free and let the GC do its work. Whatever, I've 
solved my problems by nulling out the value and calling a cleanup 
routine before the next entry is added.

         private static void cleanObjects()
         {
             if (shouldCleanObjects) {
                 foreach (key, val; objects) {
                     if (!val) {
                     objects.remove( key);
                 }
             }
             shouldCleanObjects = false;
           }
         }

  Seems to work, but I'm not shure if it is save to remove entrys while 
iterating. Can't find any advise at this.


Gerrit Wichert
Dec 07 2013
parent "Mike Parker" <aldacron gmail.com> writes:
On Saturday, 7 December 2013 at 19:04:19 UTC, Gerrit Wichert 
wrote:


  Seems to work, but I'm not shure if it is save to remove 
 entrys while iterating. Can't find any advise at this.


It's not when you're iterating the aa directly. You'll probably 
want something like this:

auto keys = objects.keys;
foreach( i, val; objects.values ) {
     if( val is null ) {
         objects.remove( keys[ i ]);
     }
}
Dec 07 2013
prev sibling parent "H. S. Teoh" <hsteoh quickfur.ath.cx> writes:
On Sat, Dec 07, 2013 at 08:04:03PM +0100, Gerrit Wichert wrote:
[...]

         private static void cleanObjects()
         {
             if (shouldCleanObjects) {
                 foreach (key, val; objects) {
                     if (!val) {
                     objects.remove( key);
                 }
             }
             shouldCleanObjects = false;
           }
         }
 
  Seems to work, but I'm not shure if it is save to remove entrys
 while iterating. Can't find any advise at this.


[...]

It's OK if you iterate over a *copy* of the AA keys, such as what you
get with .keys. Don't do it with .byKey, because it will lead to strange
behaviour and possibly crashes if you're not careful.

In general, it's unwise to delete things from a container while
iterating over it, unless the container was specifically designed to
support that. For details, see the comments on this issue:

	https://d.puremagic.com/issues/show_bug.cgi?id=10821


T

-- 
Obviously, some things aren't very obvious.
Dec 07 2013