Sollte sich eine return-Anweisung innerhalb oder außerhalb eines Schlosses befinden?

142

Ich habe gerade festgestellt, dass ich an einer Stelle in meinem Code die return-Anweisung innerhalb des Schlosses und irgendwann außerhalb habe. Welches ist das beste?

1)

void example()
{
    lock (mutex)
    {
    //...
    }
    return myData;
}

2)

void example()
{
    lock (mutex)
    {
    //...
    return myData;
    }

}

Welches soll ich verwenden?

Patrick Desjardins
quelle
Wie wäre es mit Reflector und IL-Vergleich ;-).
Pop Catalin
6
@ Pop: fertig - beides ist in IL-Begriffen nicht besser - es gilt nur der C # -Stil
Marc Gravell
1
Sehr interessant, wow ich lerne heute etwas!
Pokus

Antworten:

192

Im Wesentlichen, was auch immer den Code einfacher macht. Single Point of Exit ist ein schönes Ideal, aber ich würde den Code nicht aus der Form bringen, um ihn zu erreichen ... Und wenn die Alternative darin besteht, eine lokale Variable (außerhalb des Schlosses) zu deklarieren, sie zu initialisieren (innerhalb des Schlosses) und Wenn Sie es dann zurückgeben (außerhalb des Schlosses), würde ich sagen, dass ein einfaches "Return Foo" innerhalb des Schlosses viel einfacher ist.

Um den Unterschied in IL zu zeigen, lassen Sie uns Folgendes codieren:

static class Program
{
    static void Main() { }

    static readonly object sync = new object();

    static int GetValue() { return 5; }

    static int ReturnInside()
    {
        lock (sync)
        {
            return GetValue();
        }
    }

    static int ReturnOutside()
    {
        int val;
        lock (sync)
        {
            val = GetValue();
        }
        return val;
    }
}

(Beachten Sie, dass ich gerne behaupten würde, dass dies ReturnInsideein einfacheres / saubereres Stück C # ist.)

Und schauen Sie sich die IL an (Release-Modus usw.):

.method private hidebysig static int32 ReturnInside() cil managed
{
    .maxstack 2
    .locals init (
        [0] int32 CS$1$0000,
        [1] object CS$2$0001)
    L_0000: ldsfld object Program::sync
    L_0005: dup 
    L_0006: stloc.1 
    L_0007: call void [mscorlib]System.Threading.Monitor::Enter(object)
    L_000c: call int32 Program::GetValue()
    L_0011: stloc.0 
    L_0012: leave.s L_001b
    L_0014: ldloc.1 
    L_0015: call void [mscorlib]System.Threading.Monitor::Exit(object)
    L_001a: endfinally 
    L_001b: ldloc.0 
    L_001c: ret 
    .try L_000c to L_0014 finally handler L_0014 to L_001b
} 

method private hidebysig static int32 ReturnOutside() cil managed
{
    .maxstack 2
    .locals init (
        [0] int32 val,
        [1] object CS$2$0000)
    L_0000: ldsfld object Program::sync
    L_0005: dup 
    L_0006: stloc.1 
    L_0007: call void [mscorlib]System.Threading.Monitor::Enter(object)
    L_000c: call int32 Program::GetValue()
    L_0011: stloc.0 
    L_0012: leave.s L_001b
    L_0014: ldloc.1 
    L_0015: call void [mscorlib]System.Threading.Monitor::Exit(object)
    L_001a: endfinally 
    L_001b: ldloc.0 
    L_001c: ret 
    .try L_000c to L_0014 finally handler L_0014 to L_001b
}

Auf IL-Ebene sind sie also identisch (ich habe etwas gelernt ;-p). Als solches ist der einzig sinnvolle Vergleich das (höchst subjektive) Gesetz des lokalen Codierungsstils ... Ich bevorzuge der ReturnInsideEinfachheit halber, aber ich würde mich auch nicht darüber aufregen.

Marc Gravell
quelle
15
Ich habe den .NET-Reflektor des (kostenlosen und ausgezeichneten) Red Gate verwendet (war: .NET-Reflektor von Lutz Roeder), aber ILDASM würde dies auch tun.
Marc Gravell
1
Einer der mächtigsten Aspekte von Reflector ist, dass Sie IL tatsächlich in Ihre bevorzugte Sprache (C #, VB, Delphi, MC ++, Chrome usw.)
zerlegen können
3
Für Ihr einfaches Beispiel bleibt die IL gleich, aber das liegt wahrscheinlich daran, dass Sie nur einen konstanten Wert zurückgeben?! Ich glaube, dass für reale Szenarien das Ergebnis unterschiedlich sein kann und parallele Threads Probleme für einander verursachen können, indem sie den Wert ändern, bevor er zurückgegeben wird, wenn sich die return-Anweisung außerhalb des Sperrblocks befindet. Gefährlich!
Torbjørn
@MarcGravell: Ich bin gerade auf Ihren Beitrag gestoßen, als ich darüber nachgedacht habe, und selbst nachdem ich Ihre Antwort gelesen habe, bin ich mir immer noch nicht sicher: Gibt es Umstände, unter denen die Verwendung des externen Ansatzes die thread-sichere Logik brechen könnte? Ich frage dies, da ich einen einzigen Rückgabepunkt bevorzuge und mich in Bezug auf die Gewindesicherheit nicht gut fühle. Obwohl, wenn die IL die gleiche ist, sollte meine Sorge sowieso strittig sein.
Raheel Khan
1
@ RaheelKhan nein, keine; Sie sind gleich. Auf IL-Ebene können Sie sich nicht ret in einer .tryRegion befinden.
Marc Gravell
42

Es macht keinen Unterschied; Sie werden beide vom Compiler in dasselbe übersetzt.

Zur Verdeutlichung wird beides effektiv in etwas mit der folgenden Semantik übersetzt:

T myData;
Monitor.Enter(mutex)
try
{
    myData= // something
}
finally
{
    Monitor.Exit(mutex);
}

return myData;
Greg Beech
quelle
1
Nun, das gilt für den Versuch / Endlich - für die Rückkehr außerhalb des Schlosses sind jedoch noch zusätzliche Einheimische erforderlich, die nicht optimiert werden können - und es wird mehr Code benötigt ...
Marc Gravell
3
Sie können nicht von einem Try-Block zurückkehren. es muss mit einem ".leave" -Op-Code enden. Die emittierte CIL sollte also in beiden Fällen gleich sein.
Greg Beech
3
Sie haben Recht - ich habe mir gerade die IL angesehen (siehe aktualisierten Beitrag). Ich habe etwas gelernt ;-p
Marc Gravell
2
Cool, leider habe ich aus schmerzhaften Stunden gelernt, als ich versuchte, .ret-Op-Codes in Try-Blöcken auszugeben und die CLR sich weigerte, meine dynamischen Methoden zu laden :-(
Greg Beech
Ich kann erzählen; Ich habe ziemlich viel nachgedacht. Geben Sie zu, aber ich bin faul; Wenn ich mir bei etwas nicht sehr sicher bin, schreibe ich repräsentativen Code in C # und schaue dann auf die IL. Es ist jedoch überraschend, wie schnell Sie anfangen, in IL-Begriffen zu denken (dh den Stapel zu sequenzieren).
Marc Gravell
28

Ich würde die Rückgabe definitiv in das Schloss stecken. Andernfalls riskieren Sie, dass ein anderer Thread in die Sperre eindringt und Ihre Variable vor der return-Anweisung ändert, sodass der ursprüngliche Aufrufer einen anderen Wert als erwartet erhält.

Ricardo Villamil
quelle
4
Dies ist richtig, ein Punkt, den die anderen Antwortenden zu vermissen scheinen. Die einfachen Proben, die sie gemacht haben, erzeugen möglicherweise dieselbe IL, dies gilt jedoch nicht für die meisten realen Szenarien.
Torbjørn
4
Ich bin
5
In diesem Beispiel geht es darum, eine Stapelvariable zum Speichern des Rückgabewerts zu verwenden, dh nur die return-Anweisung außerhalb der Sperre und natürlich die Variablendeklaration. Ein anderer Thread sollte einen anderen Stapel haben und könnte daher keinen Schaden anrichten, habe ich Recht?
Guillermo Ruffino
3
Ich denke nicht, dass dies ein gültiger Punkt ist, da ein anderer Thread den Wert zwischen dem Rückruf und der tatsächlichen Zuordnung des Rückgabewerts zur Variablen im Hauptthread aktualisieren könnte. Der zurückgegebene Wert kann in keiner Weise geändert oder garantiert werden, dass er mit dem aktuellen tatsächlichen Wert übereinstimmt. Richtig?
Uroš Joksimović
Diese Antwort ist falsch. Ein anderer Thread kann eine lokale Variable nicht ändern. Lokale Variablen werden im Stapel gespeichert, und jeder Thread hat seinen eigenen Stapel. Übrigens beträgt die Standardgröße des Stacks eines Threads 1 MB .
Theodor Zoulias
5

Es hängt davon ab, ob,

Ich werde hier gegen den Strich gehen. Ich würde im Allgemeinen innerhalb des Schlosses zurückkehren.

Normalerweise ist die Variable mydata eine lokale Variable. Ich deklariere gerne lokale Variablen, während ich sie initialisiere. Ich habe selten die Daten, um meinen Rückgabewert außerhalb meiner Sperre zu initialisieren.

Ihr Vergleich ist also tatsächlich fehlerhaft. Während der Unterschied zwischen den beiden Optionen im Idealfall so ist, wie Sie es geschrieben haben, was Fall 1 zu nicken scheint, ist es in der Praxis etwas hässlicher.

void example() { 
    int myData;
    lock (foo) { 
        myData = ...;
    }
    return myData
}

vs.

void example() { 
    lock (foo) {
        return ...;
    }
}

Ich finde Fall 2 wesentlich leichter zu lesen und schwerer zu vermasseln, insbesondere bei kurzen Ausschnitten.

Edward KMETT
quelle
4

Wenn Sie der Meinung sind, dass das Schloss draußen besser aussieht, aber seien Sie vorsichtig, wenn Sie den Code in Folgendes ändern:

return f(...)

Wenn f () mit gehaltenem Schloss aufgerufen werden muss, muss es sich offensichtlich innerhalb des Schlosses befinden, da es sinnvoll ist, die Rückgabe aus Gründen der Konsistenz innerhalb des Schlosses zu halten.

Rob Walker
quelle
1

In der Dokumentation zu MSDN finden Sie ein Beispiel für die Rückkehr aus dem Inneren des Schlosses. Von den anderen Antworten hier scheint es IL ziemlich ähnlich zu sein, aber für mich scheint es sicherer, aus dem Schloss zurückzukehren, da Sie dann nicht das Risiko eingehen, dass eine Rückgabevariable von einem anderen Thread überschrieben wird.

greyseal96
quelle
0

Um anderen Entwicklern das Lesen des Codes zu erleichtern, würde ich die erste Alternative vorschlagen.

Adam Asham
quelle
0

lock() return <expression> Aussagen immer:

1) Sperre eingeben

2) macht einen lokalen (thread-sicheren) Speicher für den Wert des angegebenen Typs,

3) füllt das Geschäft mit dem von zurückgegebenen Wert <expression>,

4) Ausgangssperre

5) Geben Sie den Laden zurück.

Dies bedeutet, dass der von der lock-Anweisung zurückgegebene Wert vor der Rückgabe immer "gekocht" wird.

Mach dir keine Sorgen lock() return, hör hier niemandem zu))

mshakurov
quelle
-2

Hinweis: Ich glaube, dass diese Antwort sachlich korrekt ist, und ich hoffe, dass sie auch hilfreich ist, aber ich bin immer froh, sie auf der Grundlage konkreter Rückmeldungen zu verbessern.

Um die vorhandenen Antworten zusammenzufassen und zu ergänzen:

  • Die akzeptierte Antwort zeigt, dass unabhängig davon, welche Syntaxform Sie in Ihrem C # -Code wählen , im IL-Code - und damit zur Laufzeit - returndies erst nach dem Aufheben der Sperre geschieht.

    • Obwohl das Platzieren des return Inneren des lockBlocks genau genommen den Kontrollfluss falsch darstellt [1] , ist es syntaktisch praktisch , da die Notwendigkeit, den Rückgabewert in einem Aux zu speichern, entfällt. lokale Variable (außerhalb des Blocks deklariert, damit sie mit einer returnaußerhalb des Blocks verwendet werden kann) - siehe Antwort von Edward KMETT .
  • Getrennt - und dieser Aspekt ist nebensächlich auf die Frage, aber immer noch von Interesse (sein kann Ricardo Villamil Antwort versucht es zu lösen, aber falsch, glaube ich) - eine Kombination lockAnweisung mit einer returnAussage - dh Wert zu erhalten , um returnin einem Block geschützt von die gleichzeitige Zugriff - nur dann sinnvoll „schützt“ den Rückgabewert in dem Anrufer ‚s Umfang , wenn es tatsächlich nicht einmal erhalten muß schütz , die in den folgenden Szenarien gelten:

    • Wenn der zurückgegebene Wert ein Element aus einer Sammlung ist, das nur durch Hinzufügen und Entfernen von Elementen geschützt werden muss , nicht durch Änderungen der Elemente selbst und / oder ...

    • ... wenn der zurückgegebene Wert eine Instanz eines Werttyps oder einer Zeichenfolge ist .

      • Beachten Sie, dass der Anrufer in diesem Fall eine Momentaufnahme (Kopie) [2] des Werts erhält, die zum Zeitpunkt der Überprüfung durch den Anrufer möglicherweise nicht mehr der aktuelle Wert in der Ursprungsdatenstruktur ist.
    • In jedem anderen Fall muss die Sperre vom Aufrufer durchgeführt werden , nicht (nur) innerhalb der Methode.


[1] Theodor Zoulias weist darauf hin , dass das technisch gilt auch für die Platzierung returninnerhalb try, catch, using, if, while, for, ... Aussagen; Es ist jedoch der spezifische Zweck der lockErklärung, der wahrscheinlich zu einer Überprüfung des tatsächlichen Kontrollflusses einlädt, wie aus dieser Frage hervorgeht, die gestellt wurde und viel Aufmerksamkeit erhalten hat.

[2] Durch den Zugriff auf eine Instanz vom Werttyp wird ausnahmslos eine threadlokale Kopie davon auf dem Stapel erstellt. Obwohl Zeichenfolgen technisch Instanzen vom Referenztyp sind, verhalten sie sich effektiv wie Instanzen vom Typ "Wert".

mklement0
quelle
In Bezug auf den aktuellen Status Ihrer Antwort (Revision 13) spekulieren Sie immer noch über die Gründe für das Vorhandensein der lockund leiten die Bedeutung aus der Platzierung der Rückgabeerklärung ab. Welches ist eine Diskussion ohne Bezug zu dieser Frage IMHO. Auch finde ich die Verwendung von "falsch dargestellt" ziemlich störend. Wenn Rückkehr von einer lockmisrepresents des Ablaufs der Steuerung, dann könnte dieselbe von einem für die Rückführung gesagt werden try, catch, using, if, while, for, und anderes Konstrukt der Sprache. Es ist, als würde man sagen, dass das C # mit falschen Darstellungen des Kontrollflusses durchsetzt ist. Jesus ...
Theodor Zoulias
"Es ist, als würde man sagen, dass das C # mit falschen Darstellungen des Kontrollflusses durchsetzt ist" - Nun, das ist technisch richtig, und der Begriff "falsche Darstellung" ist nur dann ein Werturteil, wenn Sie sich dafür entscheiden, es so zu verstehen. Mit try,, if... Ich persönlich neige nicht dazu, darüber nachzudenken, aber im Zusammenhang mit lockspeziell der Frage stellte sich für mich - und wenn sie nicht auch für andere aufgetaucht wäre, wäre diese Frage niemals gestellt worden und die akzeptierte Antwort hätte nicht große Anstrengungen unternommen, um das wahre Verhalten zu untersuchen.
mklement0