Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tag destructor throws an exception with an incorrect IP #432

Closed
VitorKawao opened this issue Jan 24, 2025 · 10 comments
Closed

Tag destructor throws an exception with an incorrect IP #432

VitorKawao opened this issue Jan 24, 2025 · 10 comments

Comments

@VitorKawao
Copy link

VitorKawao commented Jan 24, 2025

Hello,

I am using libplctag at version 1.6.0-alpha.2 and libplctag.NativeImport at version 2.0.0.0-alpha.5

I am trying to connect to Compact Logix PLC (Allen Bradley).
It works fine when the IP is correct, but when I use an invalid IP or the network is down, it generates an exception in the Tag destructor. (ErrNullPtr)

Image

When I call tag.Initialize(); with an incorrect IP, it throws an exception that I can handle. However, after some time, the destructor is called asynchronously, and it throws the same exception again, causing my app to crash.

Is there a better way to check for a connection than using tag.Initialize()? Or is it possible to handle this exception?

I am attaching my debug logs

log.txt

Thank you,
Vítor Guedes

@kyle-github
Copy link
Member

Is that throwing inside the destructor? Once the core library API call plc_tag_destroy() is called, the tag handle does not point to anything so trying to get the status on it may result in a NULL pointer warning from inside the library. It is not an actually NULL pointer reference, it is that the library noticed that it had a null pointer instead of a valid tag info object.

@timyhac
Copy link
Collaborator

timyhac commented Jan 25, 2025

Yep there are definitely some bugs here, thanks for raising it @VitorKawao.

There was some work done in #420 that introduced this issue and I'm surprised this hasn't surfaced before.

Below are some repros of the problem.

Test Harness

using libplctag;
using System;
using System.Diagnostics;

var tag = new Tag()
{
    Gateway = "10.10.10.10",
    Path = "1,0",
    PlcType = PlcType.ControlLogix,
    Protocol = Protocol.ab_eip,
    Name = "FAKE_DINT",
    Timeout = TimeSpan.FromSeconds(5)
};

var sw = new Stopwatch();
sw.Start();

Log("Start");

// ... Insert Test Case ...

Log("Finish");

void Log(string message) => Console.WriteLine($"[{sw.ElapsedMilliseconds:0000}ms] {message}");

Case 1: `InitializeAsync()` then `Dispose()`

Expectation is to see InitializeAsync() throw with Timeout after 5 seconds, and then the Dispose succesfully completes and does not perform any cleanup.

try
{
    Log("Init");
    await tag.InitializeAsync();
    Log("Init Success");
}
catch (Exception ex)
{
    Log($"Init Exception: {ex.GetType()} {ex.Message}");
}

try
{
    Log("Dispose");
    tag.Dispose();
    Log("Dispose Success");
}
catch (Exception ex)
{
    Log($"Dispose Exception: {ex.GetType()} {ex.Message}");
}
[0000ms] Start
[0015ms] Init
[5067ms] Init Exception: libplctag.LibPlcTagException ErrorTimeout
[5067ms] Dispose
[5069ms] Dispose Success
[5070ms] Finish

Case 2: `InitializeAsync(token)` then `Dispose()`, where token cancels after 1 second

Expectation is to see `InitializeAsync()` throw with `TaskCancelledException` after 1 second, and then the Dispose succesfully completes and does not perform any cleanup.

try
{
    Log("Init");
    var cts = new CancellationTokenSource();
    cts.CancelAfter(TimeSpan.FromSeconds(1));
    await tag.InitializeAsync(cts.Token);
    Log("Init Success");
}
catch (Exception ex)
{
    Log($"Init Exception: {ex.GetType()} {ex.Message}");
}

try
{
    Log("Dispose");
    tag.Dispose();
    Log("Dispose Success");
}
catch (Exception ex)
{
    Log($"Dispose Exception: {ex.GetType()} {ex.Message}");
}
[0000ms] Start
[0010ms] Init
[1067ms] Init Exception: System.Threading.Tasks.TaskCanceledException A task was canceled.
[1067ms] Dispose
[1099ms] Dispose Success
[1099ms] Finish

Case 3: `Initialize()` then `Dispose()`

Expectation is to see Initialize() throw with Timeout after 5 seconds, and then the Dispose succesfully completes and does not perform any cleanup.

try
{
    Log("Init");
    tag.Initialize();
    Log("Init Success");
}
catch (Exception ex)
{
    Log($"Init Exception: {ex.GetType()} {ex.Message}");
}

try
{
    Log("Dispose");
    tag.Dispose();
    Log("Dispose Success");
}
catch (Exception ex)
{
    Log($"Dispose Exception: {ex.GetType()} {ex.Message}");
}
[0000ms] Start
[0046ms] Init
[5146ms] Init Exception: libplctag.LibPlcTagException ErrorNotFound
[5146ms] Dispose
[5153ms] Dispose Exception: libplctag.LibPlcTagException ErrorNullPtr
[5153ms] Finish

Case 4: `Dispose()` without `Initialize()` or `InitializeAsync()`

Even though it would be unusual to dispose the tag without initializing, it is possible, and the Dispose should still complete succesfully without doing any cleanup.

try
{
    Log("Dispose");
    tag.Dispose();
    Log("Dispose Success");
}
catch (Exception ex)
{
    Log($"Dispose Exception: {ex.GetType()} {ex.Message}");
}
[0000ms] Start
[0011ms] Dispose
[0037ms] Dispose Exception: libplctag.LibPlcTagException ErrorNullPtr
[0038ms] Finish

Results

Case 1: PASS
Case 2: PASS
Case 3: FAIL
Case 4: FAIL

@timyhac
Copy link
Collaborator

timyhac commented Jan 25, 2025

PR #433 shuffles the logic around to resolve the above test cases.

@VitorKawao
Copy link
Author

Thank you for the quick reply.
Would you like me to do some tests?

Do you intend to generate libplctag-v1.6.0-alpha.3 after this fix?

Thank you,
Vítor Guedes

@timyhac
Copy link
Collaborator

timyhac commented Jan 27, 2025

Do you intend to generate libplctag-v1.6.0-alpha.3 after this fix?

Yes

Would you like me to do some tests?

Yes I would appreciate if could test with the permutations of synchronous and asynchronous initialization, explicit and implicit disposal, and plcs/tags that exist and those that don't,

Explicit Disposal

var cts = new CancellationTokenSource();
cts.CancelAfter(60*1000); // 1 minute

while(!cts.IsCancellationRequested)
{
  var tag = new Tag()
  {
      ...
  };
  
  tag.Initialize(); // also test with `await tag.InitializeAsync()`
  tag.Dispose();
}

Implicit Disposal

while(!cts.IsCancellationRequested)
{
  var tag = new Tag()
  {
      ...
  };
  
  tag.Initialize();  // also test with `await tag.InitializeAsync()`
  // tag.Dispose();
}

@VitorKawao
Copy link
Author

Hello,

I tested using the libplctag.dll generated by #433, and it seems to work fine for both my bug and the two tests mentioned above.
Even if the PLC/tag does not exist, it is not throwing an exception

Best regards,
Vítor Guedes

@timyhac
Copy link
Collaborator

timyhac commented Jan 29, 2025

Thanks! Released as v1.6.0-alpha.3

@timyhac timyhac closed this as completed Jan 29, 2025
@kyle-github
Copy link
Member

Can we close this?

@timyhac
Copy link
Collaborator

timyhac commented Jan 30, 2025

@kyle-github - I closed it, is there any work remaining?

@kyle-github
Copy link
Member

Sorry, missed that it was already closed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants