Laden...

Bug in LightCore? (ThreadSingletonLifecycle)

Erstellt von mouk vor 13 Jahren Letzter Beitrag vor 13 Jahren 7.751 Views
M
mouk Themenstarter:in
9 Beiträge seit 2010
vor 13 Jahren
Bug in LightCore? (ThreadSingletonLifecycle)

Hallo zusammen,

in der Klasse ThreadSingletonLifecycle habe ich den folgenden Bug entdeckt.

Objekte mit dem o.g. LiftCycle werden, nachdem der Thread terminiert wurde, nicht freigegeben. Habe dazu den Folgenenden Unittest gebastelt:

[TestMethod()]
public void ThreadSingletonShouldBecollectedwhenThreadFinished()
{
	var builder = new ContainerBuilder();			
	builder.Register<IDo, Do>().ControlledBy<ThreadSingletonLifecycle>();
	var container = builder.Build();
	WeakReference obj = null; ;
	Action action = () =>
	{
		var o = container.Resolve<IDo>();
		obj = new WeakReference(o);
	};

	var thread = new Thread(new ThreadStart(action));
	thread.Start();
	thread.Join();
	Assert.IsTrue(obj.IsAlive);
	thread = null;
	GC.Collect(2);
	Assert.IsFalse(obj.IsAlive, "Objekt sollte nicht mehr existieren");		
}


public interface IDo{}

public class Do : IDo{}

Dies liegt daran, dass die Objekte in einem Dictionary gespeichert werden, das für die gesamte Lebenszeit des Containers erhalten bleibt. Dies kann man lösen, indem man das Objekte als static markiert und mit der Attribute [ThreadStatic] versieht.

Viele Grüße,

Mouk

1.044 Beiträge seit 2008
vor 13 Jahren

Hallo mouk,

_action _muss auch null sein, damit der GC keine Referenzen auf _obj _bzw. die Instanz von Do hat.

zero_x

M
mouk Themenstarter:in
9 Beiträge seit 2010
vor 13 Jahren

action enthält nur einen Verweis auf container, was aber nicht schlimm ist. o ist nur eine lokale Variable, die bei jedem Aufruf angelegt wird und danach sofort zum GCen freigegeben wird.

Obj ist eine WeakReference. D.H. die hindert dem GC nicht daran, ihren Inhalt zu löschen (Das ist die einzige Aufgabe von WeakReference: Einen Verweis auf ein Objekt liefern, solange dieses Objekt noch lebt).

Von daher ist der Test korrekt.

Mouk

5.941 Beiträge seit 2005
vor 13 Jahren

Hallo mouk

Der Bug ist gefixt und ins SVN Repository eingecheckt.
Wenn alle bekannte Bugs gefixt worden sind, gibt es einen neuen Release.

Fixed ThreadSingleton does not release references after thread exit bug.
see: (Bug in LightCore? (ThreadSingletonLifecycle)).

Beschreibung:
Ich habe das Problem so gelöst, das Lifecycle-intern nicht das Object (object) referenziert wird (Im Dictionary gehalten), sondern eine WeakReference davon.

Das wars schon 😃

Gruss Peter

--
Microsoft MVP - Visual Developer ASP / ASP.NET, Switzerland 2007 - 2011

M
mouk Themenstarter:in
9 Beiträge seit 2010
vor 13 Jahren

Hallo Peter,

danke für den Bug-fix. Ich glaube jedoch, dass jetz die Singleton-Eigenschaft verletzt wird.

Falls du zweie Methoden hast, M1() und M2(), die jeweils

container.Resolve<Object>(); aufrufen, dann gibt es keine Garantie mehr, dass sie das gleiche Objekt bekommen, auch wenn sie im gleichen Thread laufen. Nach dem Aufruf von M1() könnte der GC anspringen und das Objekt entfernen. Der M2()-Aufruf (oder ein weiter M1-Aufruf ) würde ein neues Objekt bekommen.

Außerdem müsste man jetzt einen Prozess regelmäßig starten, der die toten WeaklReferences aus dem Dictionary entfernt (und dabei das Dictionary auch lockt).

Das Dictionary folgendermaßen definieren

[ThreadStatic]
private static Dictionary<Type, Object> _dictionary = new ..;

würde das problem einfach lösen.

5.941 Beiträge seit 2005
vor 13 Jahren

Hallo mouk

Das ursprüngliche Beispiel ist in meinen Augen fehlerhaft.
Wenn die Variable als Thread-affines Singleton definiert wird, ist es ja per se verboten, diese in mehr als zwei Threads zu nutzen.

Wenn Du das - per Tricks wie hier mit einer Captured Variable - doch machst, hebelst Du die Eigenschaft der Thread-Affinität aus und "missbrauchst" LightCore ja quasi. Dass das nicht funktioniert, ist klar.

Ist in meinen Augen aber kein Bug, weil falsche Voraussetzungen gegeben sind.

(Wenn ich auf ein Objekt in einer WeakReference noch eine zusätzliche Referenz halte, wird das auch nicht richtig abgeräumt - da kann die WeakReference dann aber auch nichts für 😉)

Auch ist es in meinen Augen nicht klar, ob der Thread mit dem Setzen auf null und einem Absetzen von GC.Collect(2) wirklich abgeräumt wird.

Ein Thread.Abort() würde diese Aufgabe vermutlich sauberer lösen.

In meinen Augen ist das kein offensichtlicher Fehler, da das Beispiel relativ konstruiert ist und das fehlende Abräumen von Instanzen nicht direkt als Fehler sichtbar ist.

Gruss Peter

--
Microsoft MVP - Visual Developer ASP / ASP.NET, Switzerland 2007 - 2011

M
mouk Themenstarter:in
9 Beiträge seit 2010
vor 13 Jahren

Hallo Peter,

das Beispiel ist natürlich konstruiert, es soll ja nur den Bug demonstrieren und nicht die best practices vorstellen. (Trotzdem ist es nicht verboten, eine ThreadSingle Variablen in einem fremden Thread zu verwenden, es mach nur i.d.R keinen Sinn)

Bei diesem Bug handelt es sich einfach darum, dass instanziiere Objekte nicht freigegeben werden, nachdem der dazugehörige Thread terminiert wurde.

Die captured Variable dient einfach der Demonstration. Du kannst das fehlerhafte Verfahren beobachten, indem du eine Instanz in einem neuen Thread holst, den Thread terminiert und dann in einem Profiler den Speicher untersuchen analysiert, oder einfach in der Klasse Do einen Verweis auf ein 200 MB großes Array verwalten und nach dem Aufruf von DC.Collect dir den Speicherverbrauch in dem Task-Manager anschaust. Daraus lässt sich aber kein Unitest ableiten.

Thread.Abort() ist an dieser Stelle, meiner Meinung nach, nicht wichtig, da Thread.Join aufgerufen wurde. Damit ist der Zustand des Threads Stopped. Man kann aber trotzdem Abort() zusätzlich aufrufen, ändert auch nichts am Ergebnis.

Gruß,

Mouk

P.S. ich hab den exakten Unitest erfolgreich mit StructureMap ausgeführt. Man könnte sich weitere IoC Containers anschauen.