commit c5c64bd98989074523e83a395b8efe7358e8c05d
Author: Alan McGovern <alan.mcgov...@gmail.com>
Date: Sun Sep 12 23:21:07 2010 +0100
fix a pretty bad bug accessing the g_lists from managed code
If managed land created a GLib.List from the native pointer and then we
did something to cause an item to be added/removed from the native list,
the managed copy would be out of sync and would be pointing at
just-freed memory. For example if the first item in the glist was
removed in native code, the managed copy would still think it was there.
We work around this issue by instantiating a GLib.List and letting it
die as soon as we use it once. We don't store it and re-use it.
Signed-off-by: Nathaniel McCallum <nathan...@natemccallum.com>
bindings/mono/libgpod-sharp/ChapterData.cs | 6 ++++
bindings/mono/libgpod-sharp/GPodList.cs | 39 +++++++++++++++------------
bindings/mono/libgpod-sharp/ITDB.cs | 12 ++++++++
bindings/mono/libgpod-sharp/PhotoAlbum.cs | 5 +++
bindings/mono/libgpod-sharp/PhotoDB.cs | 10 +++++++
bindings/mono/libgpod-sharp/Playlist.cs | 6 ++++
6 files changed, 61 insertions(+), 17 deletions(-)
---
diff --git a/bindings/mono/libgpod-sharp/ChapterData.cs
b/bindings/mono/libgpod-sharp/ChapterData.cs
index 3bff39b..de40136 100644
--- a/bindings/mono/libgpod-sharp/ChapterData.cs
+++ b/bindings/mono/libgpod-sharp/ChapterData.cs
@@ -59,6 +59,12 @@ namespace GPod {
this[index].SetBorrowed(false); // We're creating a new
object here, so just deallocate the old one
Itdb_ChapterData.itdb_chapterdata_add_chapter(handle,
item.StartPosition, item.Title);
}
+
+ protected unsafe override GLib.List List {
+ get {
+ return new GLib.List (((Itdb_ChapterData *)
handle.Handle)->chapters, typeof (Chapter));
+ }
+ }
}
public unsafe class ChapterData : GPodBase {
diff --git a/bindings/mono/libgpod-sharp/GPodList.cs
b/bindings/mono/libgpod-sharp/GPodList.cs
index df553c0..6734171 100644
--- a/bindings/mono/libgpod-sharp/GPodList.cs
+++ b/bindings/mono/libgpod-sharp/GPodList.cs
@@ -20,38 +20,43 @@ namespace GPod {
using System;
using System.Collections;
using System.Collections.Generic;
+ using System.Linq;
using System.Runtime.InteropServices;
using GLib;
internal abstract class GPodList<T> : IList<T> where T : IGPodBase {
- private class GPodListEnumerator : IEnumerator<T> {
- private System.Collections.IEnumerator enumerator;
- public
GPodListEnumerator(System.Collections.IEnumerator enumerator) { this.enumerator
= enumerator; }
- public T Current { get {
return (T) enumerator.Current; } }
- object IEnumerator.Current { get {
return enumerator.Current; } }
- public bool MoveNext() { return
enumerator.MoveNext(); }
- public void Reset() {
enumerator.Reset(); }
- public void Dispose() { }
- }
protected HandleRef handle;
- protected List list;
+ protected abstract GLib.List List {
+ get;
+ }
protected bool owner;
- public GPodList(bool owner, HandleRef handle, List list) {
this.handle = handle; this.list = list; this.owner = owner; }
+ public GPodList(bool owner, HandleRef handle, List list) {
this.handle = handle; this.owner = owner; }
public GPodList(HandleRef handle, List list) : this(false,
handle, list) {}
- public GPodList(bool owner, HandleRef handle, IntPtr listp) :
this(owner, handle, new List(listp, typeof(T))) {}
+ public GPodList(bool owner, HandleRef handle, IntPtr listp) :
this(owner, handle, null) {}
public GPodList(HandleRef handle, IntPtr listp) : this(false,
handle, listp) {}
- public int Count { get { return
list.Count; } }
+ public int Count { get { return
List.Count; } }
public bool IsReadOnly { get { return false; } }
- public T this[int index] { get { return (T) list[index];
}
+ public T this[int index] { get { return (T) List[index];
}
set {
RemoveAt(index); Insert(index, value); } }
public void Add(T item) { DoAdd (-1, item); if (owner)
item.SetBorrowed(true); }
- public void Clear() { list.Empty(); }
- public void CopyTo(T[] array, int arrayIndex) {
list.CopyTo(array, arrayIndex); }
- public IEnumerator<T> GetEnumerator() { return new
GPodListEnumerator(list.GetEnumerator()); }
+ public void Clear() { /* Ensure we invoke DoUnlink */ while
(Count > 0) RemoveAt (0); }
+ public void CopyTo(T[] array, int arrayIndex) {
List.CopyTo(array, arrayIndex); }
+ public IEnumerator<T> GetEnumerator()
+ {
+ // Make an explicit copy of the list here to avoid
memory
+ // corruption issues. What was happening is that
managed land was
+ // instantiating a GLib.List from the native pointer,
then if an
+ // item was unlinked from the native list, the GList in
managed
+ // land would now be pointing at freed memory and would
randomly
+ // blow up or crash. We work around this by
instantiating a
+ // GLib.List every time and copying everything out
immediately.
+ foreach (T t in List.Cast<T> ().ToArray ())
+ yield return t;
+ }
IEnumerator IEnumerable.GetEnumerator() { return
GetEnumerator(); }
public bool Contains(T item) {
diff --git a/bindings/mono/libgpod-sharp/ITDB.cs
b/bindings/mono/libgpod-sharp/ITDB.cs
index a1e0b80..6643edd 100644
--- a/bindings/mono/libgpod-sharp/ITDB.cs
+++ b/bindings/mono/libgpod-sharp/ITDB.cs
@@ -100,12 +100,24 @@ namespace GPod {
public ITDBTrackList(bool owner, HandleRef handle, IntPtr list)
: base(owner, handle, list) {}
protected override void DoAdd(int index, Track item) {
Itdb_iTunesDB.itdb_track_add(this.handle, item.Handle, index); }
protected override void DoUnlink(int index) {
Itdb_iTunesDB.itdb_track_unlink(this[index].Handle); }
+ protected unsafe override GLib.List List {
+ get {
+ return new GLib.List (((Itdb_iTunesDB *)
handle.Handle)->tracks, typeof (Track));
+ }
+ }
+
}
internal class ITDBPlaylistList : GPodList<Playlist> {
public ITDBPlaylistList(bool owner, HandleRef handle, IntPtr
list) : base(owner, handle, list) {}
protected override void DoAdd(int index, Playlist item) {
Itdb_iTunesDB.itdb_playlist_add(this.handle, item.Handle, index); }
protected override void DoUnlink(int index) {
Itdb_iTunesDB.itdb_playlist_unlink(this[index].Handle); }
+
+ protected unsafe override GLib.List List {
+ get {
+ return new GLib.List(((Itdb_iTunesDB *)
handle.Handle)->playlists, typeof (Playlist));
+ }
+ }
}
public unsafe class ITDB : GPodBase {
diff --git a/bindings/mono/libgpod-sharp/PhotoAlbum.cs
b/bindings/mono/libgpod-sharp/PhotoAlbum.cs
index 7db5aeb..3803176 100644
--- a/bindings/mono/libgpod-sharp/PhotoAlbum.cs
+++ b/bindings/mono/libgpod-sharp/PhotoAlbum.cs
@@ -65,6 +65,11 @@ namespace GPod {
Itdb_PhotoAlbum.itdb_photodb_photoalbum_add_photo(photodb, this.handle,
item.Handle, index);
}
protected override void DoUnlink(int index) {
Itdb_PhotoDB.itdb_photodb_photoalbum_unlink(this[index].Handle); }
+ protected unsafe override GLib.List List {
+ get {
+ return new GLib.List (((Itdb_PhotoAlbum *)
handle.Handle)->members, typeof (Artwork));
+ }
+ }
}
public unsafe class PhotoAlbum : GPodBase {
diff --git a/bindings/mono/libgpod-sharp/PhotoDB.cs
b/bindings/mono/libgpod-sharp/PhotoDB.cs
index 99b3cac..7bd53be 100644
--- a/bindings/mono/libgpod-sharp/PhotoDB.cs
+++ b/bindings/mono/libgpod-sharp/PhotoDB.cs
@@ -75,12 +75,22 @@ namespace GPod {
public PhotoDBArtworkList(bool owner, HandleRef handle, IntPtr
list) : base(owner, handle, list) {}
protected override void DoAdd(int index, Artwork item) { } //
TODO: How do I make itdb_photodb_add_photo() fit this convention?
protected override void DoUnlink(int index) { } // TODO: How do
I make itdb_photodb_remove_photo() fit this convention?
+ protected unsafe override GLib.List List {
+ get {
+ return new GLib.List (((Itdb_PhotoDB *)
handle.Handle)->photos, typeof (Artwork));
+ }
+ }
}
internal class PhotoDBPhotoAlbumList : GPodList<PhotoAlbum> {
public PhotoDBPhotoAlbumList(bool owner, HandleRef handle,
IntPtr list) : base(owner, handle, list) {}
protected override void DoAdd(int index, PhotoAlbum item) {
Itdb_PhotoDB.itdb_photodb_photoalbum_add(this.handle, item.Handle, index); }
protected override void DoUnlink(int index) {
Itdb_PhotoDB.itdb_photodb_photoalbum_unlink(this[index].Handle); }
+ protected unsafe override GLib.List List {
+ get {
+ return new GLib.List (((Itdb_PhotoDB *)
handle.Handle)->photoalbums, typeof (PhotoAlbum));
+ }
+ }
}
public unsafe class PhotoDB : GPodBase {
diff --git a/bindings/mono/libgpod-sharp/Playlist.cs
b/bindings/mono/libgpod-sharp/Playlist.cs
index 7a5e252..b89cd33 100644
--- a/bindings/mono/libgpod-sharp/Playlist.cs
+++ b/bindings/mono/libgpod-sharp/Playlist.cs
@@ -94,6 +94,12 @@ namespace GPod {
public PlaylistTrackList(HandleRef handle, IntPtr list) :
base(handle, list) {}
protected override void DoAdd(int index, Track item) {
Itdb_Playlist.itdb_playlist_add_track(this.handle, item.Handle, index); }
protected override void DoUnlink(int index) {
Itdb_Playlist.itdb_playlist_remove_track(this.handle, this[index].Handle); }
+
+ protected unsafe override GLib.List List {
+ get {
+ return new GLib.List (((Itdb_Playlist *)
handle.Handle)->members, typeof (Track));
+ }
+ }
}
public unsafe class Playlist : GPodBase {
------------------------------------------------------------------------------
Start uncovering the many advantages of virtual appliances
and start using them to simplify application deployment and
accelerate your shift to cloud computing.
http://p.sf.net/sfu/novell-sfdev2dev
_______________________________________________
gtkpod-cvs2 mailing list
gtkpod-cvs2@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/gtkpod-cvs2
