IDisposable für Singleton

  • C#
  • .NET (FX) 4.5–4.8

Es gibt 17 Antworten in diesem Thema. Der letzte Beitrag () ist von Gonger96.

    IDisposable für Singleton

    Guten Tag,
    ich habe in meinem Projekt Hardware auf die ich über USB (mittels LibUSB) aus C# heraus zugreife. Der Zugriff klappt soweit auch. Jetzt habe ich eine die entsprechende Klasse dafür als Singleton vorgesehen. Zusätzlich soll die aber auch IDisposable implementieren um die Verbindung zu schließen.

    Spoiler anzeigen

    C#-Quellcode

    1. public class UsbMotorDriver : IDisposable
    2. {
    3. const string libName = "libusb0.dll";
    4. const short vid = 0x16c0, pid = 0x05dc;
    5. IntPtr usbDevHandle = IntPtr.Zero;
    6. // Es folgen diverse P/Invoke Calls
    7. static UsbMotorDriver usbDriver = null;
    8. public static UsbMotorDriver Instance
    9. {
    10. get
    11. {
    12. if (usbDriver == null)
    13. usbDriver = new UsbMotorDriver();
    14. return usbDriver;
    15. }
    16. }
    17. public void ConnectHardware()
    18. {
    19. usbDevHandle = ...
    20. }
    21. public bool TryConnectHardware()
    22. {
    23. usbDevHandle = ...
    24. }
    25. public unsafe byte[] SendRequest(UsbRequestType request, int value, int index)
    26. {
    27. ...
    28. }
    29. public void DisconnectHardware()
    30. {
    31. ...
    32. usbDevHandle = IntPtr.Zero;
    33. }
    34. private bool disposedValue = false;
    35. protected virtual void Dispose(bool disposing)
    36. {
    37. if (!disposedValue)
    38. {
    39. if (disposing)
    40. usbDriver = null;
    41. if (usbDevHandle != IntPtr.Zero)
    42. {
    43. UsbClose(usbDevHandle);
    44. usbDevHandle = IntPtr.Zero;
    45. }
    46. disposedValue = true;
    47. }
    48. }
    49. ~UsbMotorDriver()
    50. {
    51. Dispose(false);
    52. }
    53. public void Dispose()
    54. {
    55. Dispose(true);
    56. GC.SuppressFinalize(this);
    57. }
    58. };

    Eigentlich müsste ich so doch im gesamten Programm auf ​UsbMotorDriver.Instance zugreifen können ohne jemals Dispose aufrufen zu müssen. Das wird letztlich durch den Finalizer am Programmende aufgerufen und schließt die Usb-Verbindung. Liege ich da richtig, oder kann es passieren, dass der GC gegen meinen Willen zwischendurch mal UsbMotorDriver.Instance zerstört und den Speicher frei gibt?

    ​Grüße
    Hi
    nein, das sollte nicht passieren, da es stets eine gültige Referenz auf die Instanz gibt. Allerdings würde ich Singletons niemals als Disposable modellieren. Wenn dann würde ich eher eine statische Methode Connect schreiben, die ein IDisposable-Objekt zurückgibt, das den Disconnect durchführt. Zusätzlich dazu würde ich mehrere Aufrufe an Connect unterstützen, falls möglich/sinnvoll.

    Viele Grüße
    ~blaze~
    Ich wollte Connect/Disconnect ganz gerne über den Singleton laufen lassen. Die Verwendung wäre dann ziemlich bequem. Wie wäre es denn wenn ich IDisposable rauswerfe und das Schließen der Verbindung nur im Destruktor aufrufe? So ist sichergestellt, dass die Verbindung immer geschlossen wird und man könnte sie auch vorher explizit schließen.

    ​Connect kann man nur einmal aufrufen. Es macht vielleicht aber Sinn Connect im SendRequest aufzurufen, falls noch nicht verbunden wurde.
    Auf welcher Ebene nutzt du denn den Singleton? Auf der höchsten wäre es okay, es über Singleton zu machen, ansonsten wäre es eher nicht meine Wahl.
    Mich stört auch das Dispose, wenn du vorhast, das Objekt aus dem Dispose-Status zu wecken. Ein mit Dispose gelöschtes Objekt ist ungültig und darf per Konvention kein Verhalten mehr aufweisen, außer ObjectDisposedExceptions zu werfen.

    Also eher dann noch eine Methode IDisposable Connect, die verwaltet, ob die Verbindung geschlossen wird und nach jeder Übertragung muss dann Dispose aufgerufen werden.

    Viele Grüße
    ~blaze~
    Das Singleton ist auf der höchsten Ebene, ist auch sinnvoll für Hardware denke ich. Die Verbindung bleibt immer offen. Was hältst du denn von so etwas?

    Spoiler anzeigen

    C#-Quellcode

    1. public class UsbMotorDriver
    2. {
    3. static UsbMotorDriver usbDriver = null;
    4. public static UsbMotorDriver Instance
    5. {
    6. get
    7. {
    8. if (usbDriver == null)
    9. usbDriver = new UsbMotorDriver();
    10. return usbDriver;
    11. }
    12. }
    13. public void ConnectHardware()
    14. {
    15. ...
    16. }
    17. public bool TryConnectHardware()
    18. {
    19. ...
    20. }
    21. public unsafe byte[] SendRequest(UsbRequestType request, int value, int index)
    22. {
    23. if (!IsConnected)
    24. ConnectHardware();
    25. ...
    26. }
    27. public void DisconnectHardware()
    28. {
    29. ...
    30. usbDevHandle = IntPtr.Zero;
    31. }
    32. public bool IsConnected { get { return usbDevHandle != IntPtr.Zero; } }
    33. ~UsbMotorDriver()
    34. {
    35. if (usbDevHandle != IntPtr.Zero)
    36. {
    37. UsbClose(usbDevHandle);
    38. usbDevHandle = IntPtr.Zero;
    39. }
    40. }
    41. private UsbMotorDriver() { }
    Es ist halt nicht threadsicher und auch fehleranfällig. Threadsicherheit ist wahrscheinlich klar. Fehleranfällig z.B. da es nicht zu jedem Disconnect ein Connect geben muss. Bei meinem Dispose-Modell kannst du sowohl Threadsicherheit, als auch fehleranfälligkeitsreduzierend arbeiten, ohne die Komplexität des Zugriffs signifikant zu erhöhen.

    Viele Grüße
    ~blaze~
    Jo Threadsicherheit stimmt, das müsste ich dann vielleicht sogar ändern. Inwiefern ist es denn Fehleranfällig? Ich kann nur 1 mal Connect aufrufen (es ist nur 1 USB-Controller), erst nach dem Disconnect aufgerufen wurde kann ich erneut eine Verbindung aufbauen. Ich hab also immer 1 Connect und 1 Disconnect, wenn nicht dann Exception. In der endgültigen Anwendung wird sowieso nur einmal verbunden und das war's. Der USB-Controller dient nur als IO-Karte und zur Ansteuerung eines Motors.
    Meine Idee wäre halt gewesen, dass Connect einen Zähler hochzählt und bei Zähler > 0 alloziiert wird. Bei Dispose wird der Zähler einmalig dekrementiert und bei 0 wird das Gerät wieder freigegeben. Zusätzlich darf nur innerhalb eines nicht-freigegebenen Objekts gearbeitet werden, d.h. zwischen Connect und Dispose.

    Mit der obersten Ebene meinte ich übrigens GUI/Dienst/... was auch immer den Rahmen dieser spezifischen Anwendung enthält.

    Viele Grüße
    ~blaze~
    Singleton und IDisposable widersprechen sich kategorisch.
    Disposen bedeutet, etwas, was nicht mehr gebraucht wird, aufzuräumen.
    Das kannste mit einem Singleton nicht machen - ein Singleton muss die gesamte Laufzeit über verfügbar sein, weil man kann ihn ja nicht durch einen neuen ersetzen.

    ich weiss jetzt nicht genau, was beabsichtigt ist, aber ich hab zB so kleine Hilfs-Objekte, mit deren Hilfe ich Connections im Using-Block öffnen und schließen kann - also ich hab mir was gebastelt, was folgende Syntax bereitstellt:

    VB.NET-Quellcode

    1. Using myConnection.EnterOpen
    2. '... myConnection nutzen
    3. End Using ' sichergestellt, dass sie auch wieder geschlossen ist
    Also ich tendiere bis jetzt zu meinem zweiten Snippet. Beim ersten Zugriff wird die Verbindung aufgebaut und bei Programmschluss ruft der GC den Finalizer auf und schließt die Verbindung. Alternativ kann man auch explizit HardwareDisconnect() aufrufen. Ist dann halt Singleton. Die Verwendung wäre ​UsbMotorDriver​.Instance.SendRequest().

    @~blaze~
    ​Jo, dein Vorschlag würde auch gut funktionieren. Welche Vorteile gegenüber meiner Klasse hätte ich denn dadurch?
    Solange du nichts mit Vererbung machst oder andere Verbindungen unterstützt, würde ich aus dem Singleton außerdem eine rein statische Klasse machen. Mit dem privaten Konstruktor sowieso.

    Der Vorteil bei meiner Methode ist, dass keiner dir durch ungültige Aufrufe an Dispose das Programm stören kann. Das, was ErfinderDesRades gepostet hat, ist genau das, was ich meinte, übrigens.
    Es ist auf jeden Fall weitaus einfacher handzuhaben und die Fehlerwahrscheinlichkeit sinkt durch das Dispose, wenn richtig implementiert (d.h. eben nur beim ersten Dispose den Zähler dekrementieren).
    Wie gesagt: Wenn du dich auf der höchsten Schicht befindest, d.h. quasi Frontend, dann hast du auch bei deiner Vorgehensweise nicht unbedingt Nachteile. Sonst eher schon.

    Viele Grüße
    ~blaze~
    So, nachdem ich hardwareseitig etliches gemacht habe, hab ich die Klasse doch noch abgeändert.
    Spoiler anzeigen

    C#-Quellcode

    1. public static class UsbManager
    2. {
    3. public static UsbDriver CreateOpen()
    4. {
    5. return new UsbHelper();
    6. }
    7. private class UsbHelper : UsbDriver { public UsbHelper() : base() { } }
    8. public class UsbDriver : IDisposable
    9. {
    10. private static int instanceCounter = 0;
    11. const string libName = "libusb0.dll";
    12. const short vid = 0x16c0, pid = 0x05dc;
    13. static IntPtr usbDevHandle = IntPtr.Zero;
    14. #region API
    15. const byte RQ_GET_INPUT = 0x2;
    16. const byte RQ_SET_POS_X = 0x4;
    17. const byte RQ_GET_POS_X = 0x5;
    18. const byte RQ_RERERENCE = 0x6;
    19. const byte RQ_SET_SPEED = 0x7;
    20. ...
    21. #endregion
    22. protected UsbDriver()
    23. {
    24. if (instanceCounter == 0)
    25. Connect();
    26. instanceCounter++;
    27. }
    28. private void Connect()
    29. {
    30. if (usbDevHandle != IntPtr.Zero)
    31. throw new InvalidOperationException();
    32. UsbInit();
    33. ...
    34. }
    35. private void Disconnect()
    36. {
    37. if (usbDevHandle == IntPtr.Zero)
    38. throw new InvalidOperationException();
    39. UsbClose(usbDevHandle);
    40. usbDevHandle = IntPtr.Zero;
    41. }
    42. private bool disposedValue = false;
    43. protected virtual void Dispose(bool disposing)
    44. {
    45. if (--instanceCounter != 0 || disposedValue)
    46. return;
    47. if (usbDevHandle != IntPtr.Zero)
    48. Disconnect();
    49. disposedValue = true;
    50. }
    51. ~UsbDriver()
    52. {
    53. Dispose(false);
    54. }
    55. public void Dispose()
    56. {
    57. Dispose(true);
    58. GC.SuppressFinalize(this);
    59. }
    60. private unsafe byte[] SendRequest(byte request, int bytesCount, int value, int index)
    61. {
    62. ...
    63. }
    64. public void ReferenceAxis()
    65. {
    66. SendRequest(RQ_RERERENCE, 0, 0, 0);
    67. }
    68. public float Position
    69. {
    70. get
    71. {
    72. byte[] data = SendRequest(RQ_GET_POS_X, 4, 0, 0);
    73. if (data == null)
    74. return 0;
    75. return BitConverter.ToSingle(data, 0);
    76. }
    77. set
    78. {
    79. byte[] data = BitConverter.GetBytes(value);
    80. SendRequest(RQ_SET_POS_X, 0, BitConverter.ToInt16(data, 2), BitConverter.ToInt16(data, 0));
    81. }
    82. }
    83. public bool IsReferenced
    84. {
    85. get
    86. {
    87. byte[] data = SendRequest(RQ_GET_INPUT, 4, 0, 0);
    88. if (data == null)
    89. return false;
    90. return (data[0] & (1<<3)) != 0;
    91. }
    92. }
    93. public float Rpm
    94. {
    95. set
    96. {
    97. byte[] data = BitConverter.GetBytes(value);
    98. SendRequest(RQ_SET_SPEED, 0, BitConverter.ToInt16(data, 2), BitConverter.ToInt16(data, 0));
    99. }
    100. }
    101. public bool PositionReached
    102. {
    103. get
    104. {
    105. byte[] data = SendRequest(RQ_GET_INPUT, 4, 0, 0);
    106. if (data == null)
    107. return false;
    108. return (data[0] & (1 << 4)) != 0;
    109. }
    110. }
    111. }
    112. }

    Kann mich garnicht mehr erinnern, wann ich zuletzt ne WriteOnly Property benutzt habe :D

    P.S. Gibt es in C# garkein friend?
    Wenn du das friend aus C++ meinst, dann nicht dass ich wüsste. Gibt halt nur internal, dass es überall innerhalb der Assemblygrenzen Zugreifbar ist(und alle angegebenen Assemblies über AssemblyInternalsVisibleTo)
    Ich wollte auch mal ne total überflüssige Signatur:
    ---Leer---
    Oder du verwendest Interlocked. In .Net verwendet man btw. recht gerne lock (bzw. SyncLock in VB.Net). Ich glaube, dass du mit lock in diesem Kontext am besten wegkommst. Mit Mutex hast du mehr Verwaltungsaufwand und auch mit Interlocked benötigst du noch einen zusätzlichen Synchronisationsmechanismus, der den Fall abdeckt, dass der Zähler 0 erreicht (würde nur die Performance steigern. Rentiert sich allerdings in diesem Fall nicht)

    Designtechnisch gibt es übrigens noch weiter ein paar Unsauberkeiten: Öffentliche nested types werden nur ungerne verwendet und eigentlich beinahe nur bei expliziten Zugehörigkeiten. Viel eher befinden sich gruppierte logische Bereiche in eigenen Namensräumen.
    In Zeile 51 solltest du außerdem --instanceCounter != 0 und disposedValue vertauschen, da sonst bei redundanten Dispose-Aufrufen stets der instanceCounter reduziert wird. Wenn du ein uint als Typ wählst, hättest du übrigens dieses Problem eher weniger. Die Logik hinter diesem Ausdruck verstehe ich übrigens insgesamt nicht ganz. Der Dispose-Vorgang soll doch eigentlich auch ausgeführt werden, wenn der Zähler nicht 0 ist, oder? Also müsste ja disposedValue auf true gesetzt werden.

    Viele Grüße
    ~blaze~

    ~blaze~ schrieb:

    Öffentliche nested types werden nur ungerne verwendet und eigentlich beinahe nur bei expliziten Zugehörigkeiten.

    Wurde geändert.

    ~blaze~ schrieb:

    Der Dispose-Vorgang soll doch eigentlich auch ausgeführt werden, wenn der Zähler nicht 0 ist, oder? Also müsste ja disposedValue auf true gesetzt werden.

    Jo stimmt, ist auch nicht richtig gewesen. Danke für den Hinweis :thumbup: