Skip to content

Commit

Permalink
Mac: Fix some memory leaks
Browse files Browse the repository at this point in the history 
- NotificationCenter observers are now added during OnLoad, removed during OnUnLoad
- Use weak references for handlers in cell views
  • Loading branch information
@cwensley
cwensley committed May 25, 2024
1 parent fbee392 commit dc8a039
Show file tree
Hide file tree
Showing 9 changed files with 140 additions and 42 deletions.
7 changes: 6 additions & 1 deletion src/Eto.Mac/Forms/Cells/CellHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,12 @@ public override bool ResignFirstResponder()
public abstract class CellHandler<TWidget, TCallback> : MacObject<NSObject, TWidget, TCallback>, Cell.IHandler, ICellHandler
where TWidget: Cell
{
public IDataColumnHandler ColumnHandler { get; set; }
WeakReference _columnHandler;
public IDataColumnHandler ColumnHandler
{
get => _columnHandler?.Target as IDataColumnHandler;
set => _columnHandler = new WeakReference(value);
}

public virtual bool Editable { get; set; }

Expand Down
9 changes: 6 additions & 3 deletions src/Eto.Mac/Forms/Cells/CheckBoxCellHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,18 @@ public override NSView GetViewForItem(NSTableView tableView, NSTableColumn table
var col = Array.IndexOf(tableView.TableColumns(), tableColumn);
view.Activated += (sender, e) =>
{
var colHandler = ColumnHandler;
if (colHandler == null)
return;
var control = (CellView)sender;
var r = (int)control.Tag;
var item = getItem(control.Item, r);
var ee = MacConversions.CreateCellEventArgs(ColumnHandler.Widget, tableView, r, col, item);
ColumnHandler.DataViewHandler.OnCellEditing(ee);
var ee = MacConversions.CreateCellEventArgs(colHandler.Widget, tableView, r, col, item);
colHandler.DataViewHandler?.OnCellEditing(ee);
SetObjectValue(item, control.ObjectValue);
control.ObjectValue = GetObjectValue(item);
ColumnHandler.DataViewHandler.OnCellEdited(ee);
colHandler.DataViewHandler?.OnCellEdited(ee);
};
}
view.Tag = row;
Expand Down
9 changes: 6 additions & 3 deletions src/Eto.Mac/Forms/Cells/ComboBoxCellHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,14 +241,17 @@ public override NSView GetViewForItem(NSTableView tableView, NSTableColumn table
};
view.Activated += (sender, e) =>
{
var colHandler = ColumnHandler;
if (colHandler == null)
return;
var control = (CellView)sender;
var r = (int)control.Tag;
var item = getItem(control.Item, r);
var cellArgs = MacConversions.CreateCellEventArgs(ColumnHandler.Widget, tableView, r, col, item);
ColumnHandler.DataViewHandler.OnCellEditing(cellArgs);
var cellArgs = MacConversions.CreateCellEventArgs(colHandler.Widget, tableView, r, col, item);
colHandler.DataViewHandler?.OnCellEditing(cellArgs);
SetObjectValue(item, control.ObjectValue);
ColumnHandler.DataViewHandler.OnCellEdited(cellArgs);
colHandler.DataViewHandler?.OnCellEdited(cellArgs);
control.ObjectValue = GetObjectValue(item);
};
view.Bind(enabledBinding, tableColumn, "editable", null);
Expand Down
24 changes: 17 additions & 7 deletions src/Eto.Mac/Forms/Cells/CustomCellHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,16 @@ public override nfloat GetPreferredWidth(object value, CGSize cellSize, int row,

public class EtoCustomCellView : NSTableCellView
{
public CustomCellHandler Handler { get; set;}
WeakReference _handler;
public CustomCellHandler Handler
{
get => _handler?.Target as CustomCellHandler;
set => _handler = new WeakReference(value);
}

public MutableCellEventArgs Args { get; set; }

public Control EtoControl { get; set; }
public Control EtoControl => Args.Control;

public bool DrawsBackground { get; set; }

Expand Down Expand Up @@ -93,13 +99,16 @@ private void ControlLostFocus(object sender, EventArgs e)
if (losingFocus)
return;

losingFocus = true;
var h = Handler;
if (h == null)
return;

losingFocus = true;
var ee = MacConversions.CreateCellEventArgs(h.ColumnHandler.Widget, null, Args.Row, Args.Column, Args.Item);
if (!h.ColumnHandler.DataViewHandler.IsCancellingEdit)
if (h.ColumnHandler.DataViewHandler?.IsCancellingEdit == false)
{
h.Callback.OnCommitEdit(h.Widget, Args);
h.ColumnHandler.DataViewHandler.OnCellEdited(ee);
h.ColumnHandler.DataViewHandler?.OnCellEdited(ee);
}
else
{
Expand All @@ -111,9 +120,11 @@ private void ControlLostFocus(object sender, EventArgs e)
private void ControlGotFocus(object sender, EventArgs e)
{
var h = Handler;
if (h == null)
return;
h.Callback.OnBeginEdit(h.Widget, Args);
var ee = MacConversions.CreateCellEventArgs(h.ColumnHandler.Widget, null, Args.Row, Args.Column, Args.Item);
h.ColumnHandler.DataViewHandler.OnCellEditing(ee);
h.ColumnHandler.DataViewHandler?.OnCellEditing(ee);
}

public override void Layout()
Expand Down Expand Up @@ -168,7 +179,6 @@ public override NSView GetViewForItem(NSTableView tableView, NSTableColumn table
{
Handler = this,
Args = args,
EtoControl = control,
Identifier = identifier,
AutoresizingMask = NSViewResizingMask.HeightSizable | NSViewResizingMask.WidthSizable
};
Expand Down
30 changes: 24 additions & 6 deletions src/Eto.Mac/Forms/Cells/TextBoxCellHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,37 +129,55 @@ public override NSView GetViewForItem(NSTableView tableView, NSTableColumn table
var col = Array.IndexOf(tableView.TableColumns(), tableColumn);
view.BecameFirstResponder += (sender, e) =>
{
var colHandler = ColumnHandler;
if (colHandler == null)
return;
var table = colHandler.DataViewHandler?.Table;
if (table == null)
return;
var control = (CellView)sender;
var r = (int)control.Tag;
var item = getItem(control.Item, r);
var ee = MacConversions.CreateCellEventArgs(ColumnHandler.Widget, tableView, r, col, item);
ColumnHandler.DataViewHandler.OnCellEditing(ee);
var ee = MacConversions.CreateCellEventArgs(colHandler.Widget, table, r, col, item);
colHandler.DataViewHandler?.OnCellEditing(ee);
};
view.EditingEnded += (sender, e) =>
{
var colHandler = ColumnHandler;
if (colHandler == null)
return;
var table = colHandler.DataViewHandler?.Table;
if (table == null)
return;
var notification = (NSNotification)sender;
var control = (CellView)notification.Object;
var r = (int)control.Tag;
var item = getItem(control.Item, r);
SetObjectValue(item, control.ObjectValue);
var ee = MacConversions.CreateCellEventArgs(ColumnHandler.Widget, tableView, r, col, item);
ColumnHandler.DataViewHandler.OnCellEdited(ee);
var ee = MacConversions.CreateCellEventArgs(colHandler.Widget, table, r, col, item);
colHandler.DataViewHandler?.OnCellEdited(ee);
control.ObjectValue = GetObjectValue(item) ?? new NSString(string.Empty);
};
bool isResigning = false;
view.ResignedFirstResponder += (sender, e) =>
{
var colHandler = ColumnHandler;
if (colHandler == null)
return;
if (isResigning)
return;
var table = colHandler.DataViewHandler?.Table;
if (table == null)
return;
isResigning = true;
var control = (CellView)sender;
var r = (int)control.Tag;
var item = getItem(control.Item, r);
SetObjectValue(item, control.ObjectValue);
var ee = MacConversions.CreateCellEventArgs(ColumnHandler.Widget, tableView, r, col, item);
ColumnHandler.DataViewHandler.OnCellEdited(ee);
var ee = MacConversions.CreateCellEventArgs(colHandler.Widget, table, r, col, item);
colHandler.DataViewHandler?.OnCellEdited(ee);
isResigning = false;
};
view.Bind(editableBinding, tableColumn, "editable", null);
Expand Down
4 changes: 2 additions & 2 deletions src/Eto.Mac/Forms/Controls/GridViewHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ public override void MouseDown(NSEvent theEvent)
public EtoTableView(GridViewHandler handler)
{
FocusRingType = NSFocusRingType.None;
DataSource = new EtoTableViewDataSource { Handler = handler };
Delegate = new EtoTableDelegate { Handler = handler };
WeakDataSource = new EtoTableViewDataSource { Handler = handler };
WeakDelegate = new EtoTableDelegate { Handler = handler };
ColumnAutoresizingStyle = NSTableViewColumnAutoresizingStyle.Uniform;
SetDraggingSourceOperationMask(NSDragOperation.All, true);
SetDraggingSourceOperationMask(NSDragOperation.All, false);
Expand Down
4 changes: 2 additions & 2 deletions src/Eto.Mac/Forms/Controls/TreeGridViewHandler.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -646,8 +646,8 @@ public override void MouseDown(NSEvent theEvent)

public EtoOutlineView(TreeGridViewHandler handler)
{
Delegate = new EtoOutlineDelegate { Handler = handler };
DataSource = new EtoDataSource { Handler = handler };
WeakDelegate = new EtoOutlineDelegate { Handler = handler };
WeakDataSource = new EtoDataSource { Handler = handler };
//HeaderView = null,
AutoresizesOutlineColumn = false;
//AllowsColumnResizing = false,
Expand Down
91 changes: 73 additions & 18 deletions src/Eto.Mac/Forms/MacBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,20 @@ public interface IMacControlHandler

void InvalidateMeasure();
}

public enum ObserverType
{
Control,
NotificationCenter
}

[Register("ObserverHelper")]
public class ObserverHelper : NSObject
{
bool isNotification;
bool isControl;
bool hasNotification;
bool hasControl;

public ObserverType Type { get; set; }

public Action<ObserverActionEventArgs> Action { get; set; }

Expand Down Expand Up @@ -50,25 +58,33 @@ public override void ObserveValue(NSString keyPath, NSObject ofObject, NSDiction
{
Action(new ObserverActionEventArgs(this, null));
}

public void AddToNotificationCenter()

public void Register()
{
if (Type == ObserverType.NotificationCenter)
AddToNotificationCenter();
else
AddToControl();
}

void AddToNotificationCenter()
{
var c = Control;
if (!isNotification && c != null)
if (!hasNotification && c != null)
{
NSNotificationCenter.DefaultCenter.AddObserver(this, selPerformAction, KeyPath, c);
isNotification = true;
hasNotification = true;
}
}

public void AddToControl()
void AddToControl()
{
var c = Control;
if (!isControl && c != null)
if (!hasControl && c != null)
{
//Console.WriteLine ("{0}: 3. Adding observer! {1}, {2}", ((IRef)this.Handler).WidgetID, this.GetType (), Control.GetHashCode ());
c.AddObserver(this, KeyPath, NSKeyValueObservingOptions.New, IntPtr.Zero);
isControl = true;
hasControl = true;
}
}

Expand All @@ -77,17 +93,17 @@ public void AddToControl()
public void Remove()
{
// we use the handle here to remove as it may have been GC'd but we still need to remove it!
if (isNotification)
if (hasNotification)
{
NSNotificationCenter.DefaultCenter.RemoveObserver(this);

isNotification = false;
hasNotification = false;
}
if (isControl)
if (hasControl)
{
//Console.WriteLine ("{0}: 4. Removing observer! {1}, {2}", ((IRef)this.Handler).WidgetID, Handler.GetType (), Control.GetHashCode ());
Messaging.void_objc_msgSend_IntPtr_IntPtr(ControlHandle, selRemoveObserverForKeyPath_Handle, Handle, KeyPath.Handle);
isControl = false;
hasControl = false;
}
}

Expand Down Expand Up @@ -147,6 +163,12 @@ public abstract class MacBase<TControl, TWidget, TCallback> : WidgetHandler<TCon
where TControl : class
where TWidget : Widget
{
/// <summary>
/// Return true to delay notification center observers, then use <see cref="RegisterDelayedNotifications"/>
/// when appropriate to register them, and <see cref="RemoveNotificationCenterObservers"/> when removed (usually on OnUnload).
/// </summary>
internal virtual bool DelayRegisterNotificationCenter => false;

protected override void Initialize()
{
base.Initialize();
Expand Down Expand Up @@ -193,7 +215,6 @@ public bool HasMethod(IntPtr selector, object control)
return ObjCExtensions.GetInstanceMethod(classHandle, selector) != IntPtr.Zero;
}


public NSObject AddObserver(NSString key, Action<ObserverActionEventArgs> action, NSObject control)
{
if (observers == null)
Expand All @@ -202,12 +223,14 @@ public NSObject AddObserver(NSString key, Action<ObserverActionEventArgs> action
}
var observer = new ObserverHelper
{
Type = ObserverType.NotificationCenter,
Action = action,
KeyPath = key,
ControlHandle = control.Handle,
Handler = this
};
observer.AddToNotificationCenter();
if (!DelayRegisterNotificationCenter)
observer.Register();
observers.Add(observer);
return observer;
}
Expand All @@ -220,16 +243,50 @@ public void AddControlObserver(NSString key, Action<ObserverActionEventArgs> act
}
var observer = new ObserverHelper
{
Type = ObserverType.Control,
Action = action,
KeyPath = key,
ControlHandle = control.Handle,
Handler = this
};
observer.AddToControl();
observer.Register();
observers.Add(observer);
}

protected override void Dispose(bool disposing)
{
RemoveAllObservers();

base.Dispose(disposing);
}

internal void RegisterDelayedNotifications()
{
if (observers != null)
{
for (int i = 0; i < observers.Count; i++)
{
var observer = observers[i];
// this will only register if not already
observer.Register();
}
}
}

internal void RemoveNotificationCenterObservers()
{
if (observers != null)
{
for (int i = 0; i < observers.Count; i++)
{
var observer = observers[i];
if (observer.Type == ObserverType.NotificationCenter)
observer.Remove();
}
}
}

internal void RemoveAllObservers()
{
if (observers != null)
{
Expand All @@ -240,8 +297,6 @@ protected override void Dispose(bool disposing)
}
observers = null;
}

base.Dispose(disposing);
}
}
}
Expand Down
Loading

0 comments on commit dc8a039

Please sign in to comment.