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

Serial Modbus master reopen may always fail after error #71

Open
rtollert opened this issue Aug 20, 2019 · 0 comments
Open

Serial Modbus master reopen may always fail after error #71

rtollert opened this issue Aug 20, 2019 · 0 comments

Comments

@rtollert
Copy link

#63 (the fix for #59) introduced a DVR ("Client Count DVR") for refcounting serial master instances, to defer closing the underlying VISA session. This was implemented with a variant FGV mapping VISA instrument name to (instance, DVR) tuple in Modbus Master.lvclass:modbus initialize API object.vi. On close.vi, the DVR is deleted once it goes to zero. However, the FGV is never informed that the DVR has been deleted.

Now, normally this is fine, because of the "Not a refnum?" check: if all masters really did close, then this will return FALSE for the VISA session, thus forcing the creation of a new DVR anyway. However, the decrement-refcount-and-close process in close.vi also wires in error in. Suppose that there is a real error wired into error in of close.vi, with one serial master open. Then the subsequent sequence of events will proceed as follows, AFAIK:

  1. Decrement Clients Count.vi will skip because it implements standard error out functionality. The refcount returned will always be zero (even if the port has been opened reentrantly).
  2. The comparison against 1 will succeed.
  3. Delete Data Value Reference will run, because it exposes normal close semantics (i.e. errors are passed through, but it always runs).
  4. Modbus Master.lvclass:Shutdown.vi will effectively skip because of the underlying code. (Careful readers will need to trust me on this; some of the underlying code is password-protected.)
  5. At this point, the DVR is invalid but the VISA session is still open.
  6. DCAF will attempt to recover the module and will eventually re-enter modbus initialize API object.vi. The variant lookup will succeed, and because the VISA session is still open, the VI will continue down the TRUE case of "serial master", where the instance and refcount DVR are reused.
  7. Increment Clients Count.vi is attempted. This will fail, because the DVR no longer exists.
  8. My understanding is that this error will cause execution to proceed more or less immediately back to close.vi. Lather, rinse, and repeat.

Note, the precise details of this issue have only been inferred by inspection; I have not yet attempted a minimal reproducing case. I have sketched out an untested fix.

@agomezni @becega

rtollert added a commit to rtollert/ModbusModules that referenced this issue Aug 21, 2019
Fix for LabVIEW-DCAF#71.

modbus initialize API object.vi:
- New design assertions:
  1. Refcounts (Clients Count DVRs) are never closed.
  2. All API objects participate in refcounting, not just serial
     masters.
  This does not affect the behavior of things that aren't serial masters,
  and dramatically simplifies logic both here and in close.vi.
- Accordingly, the FGV variant now holds (Modbus API, refcount) tuples
  instead of (Modbus Master, refcount) tuples. And it is notionally getting
  indexed by (class, address) tuples, e.g. ("serial master", VISA
  instrument), or ("TCP master", IP address, port).

close.vi:
- Unwire "error in" from Decrement Clients Count.vi and the rest of that
  shutdown chain, to ensure that it always runs. This helps preserve normal
  close semantics for this VI.
- Remove Delete DVR call to Clients Count DVR, because modbus initialize API
  object.vi is now guaranteed to hold it open. Add a brief comment to this
  effect. Reorder final Merge Errors call accordingly.

Build lookup key for configuration.vi:
- Builds the strings of (class, address) tupes which are used to index into
  the FGV variant in modbus initialize API object.vi.

- "ok?" indicator controls whether or not reusing the modbus instance from
  the FGV is permitted -- false for everything but serial masters. (Refcount
  DVRs are always reused.)
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

1 participant