libplctag / libplctag.NET

A .NET wrapper for libplctag.
https://libplctag.github.io/
Mozilla Public License 2.0
221 stars 56 forks source link

Callback seems to remain on failed InitializeAsync #418

Open dnmalenke opened 1 month ago

dnmalenke commented 1 month ago

When initializing a tag that will fail with ErrorNotFound, it seems like native callbacks are registered, but not disposable. This leads to an application crash/hang on LibPlcTag.Shutdown(). Log entry:

15:56:32 VRBProcess terminated. A callback was made on a garbage collected delegate of type 
 libplctag.NativeImport!libplctag.NativeImport.plctag+callback_func_ex::Invoke'.
--------------------------------
   at libplctag.NativeImport.NativeMethods.plc_tag_shutdown()
--------------------------------
   at libplctag.NativeImport.plctag.plc_tag_shutdown()
   at libplctag.NativeTag.plc_tag_shutdown()
   at **libplctag.LibPlcTag.Shutdown()**

Unfortunately, I was unable to replicate this on a smaller scale.

I am able to workaround the issue by calling .Initialize() instead of .InitializeAsync() or forcing the _isInitialized field to true via messy reflection before disposing:

 try
 {
     await tag.InitializeAsync();
 }
 catch
 {
     var foundF = typeof(Tag).GetField("_tag", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
     var pTag = foundF!.GetValue(tag);
     pTag.GetType().GetField("_isInitialized", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance).SetValue(pTag, true);
     tag.Dispose();

     throw;
 }

This forces the Dispose call to unregister callbacks and destroy the handle.

Any ideas? Thanks

timyhac commented 1 month ago

I haven't had a chance to sit down and review this but yes you're probably onto something. Probably need to unregister the callbacks here: https://github.com/libplctag/libplctag.NET/blob/ae44f94713b379591a1b20f274c8986f59ab63ae/src/libplctag/Tag.cs#L706.

timyhac commented 1 month ago

@dnmalenke - although it makes sense that the callbacks need to be removed, I was not able to trigger the issue with a unit test.

It also doesn't make sense that the issue wouldn't occur when using Initialize - the same callbacks are added.

Having said all this I think it still is a problem so have put together PR #420 - is it possible you could test it out to see if it resolves the problem?

dnmalenke commented 1 month ago

@timyhac Thank you, this seems to have resolved the issue.

I do want to note, that with this update, the function public void GetBuffer(byte[] buffer) stopped working. I had to use public byte[] GetBuffer() within my decoding functions instead

timyhac commented 1 month ago

Thanks for testing.

I'm not sure that the GetBuffer issue is related to this specific change - could you raise a separate issue with the normal details like a repro, debug logs, versions, etc

On Wed, 9 Oct 2024, 8:03 am David Malenke, @.***> wrote:

@timyhac https://github.com/timyhac Thank you, this seems to have resolved the issue.

I do want to note, that with this update, the function public void GetBuffer(byte[] buffer) stopped working. I had to use public byte[] GetBuffer() within my decoding functions instead

— Reply to this email directly, view it on GitHub https://github.com/libplctag/libplctag.NET/issues/418#issuecomment-2400816448, or unsubscribe https://github.com/notifications/unsubscribe-auth/AESB443CFHOTW37O7SWAE43Z2RCCBAVCNFSM6AAAAABPINCCMCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBQHAYTMNBUHA . You are receiving this because you were mentioned.Message ID: @.***>