Skip to content

fix(opcua): clean client on disconnect so that connect works cleanly#9583

Merged
sspaink merged 1 commit intoinfluxdata:masterfrom
re-gmbh:fix/opcua-client-cleanup
Aug 10, 2021
Merged

fix(opcua): clean client on disconnect so that connect works cleanly#9583
sspaink merged 1 commit intoinfluxdata:masterfrom
re-gmbh:fix/opcua-client-cleanup

Conversation

@milgner
Copy link
Copy Markdown
Contributor

@milgner milgner commented Aug 4, 2021

  • [-] Updated associated README.md.
  • [-] Wrote appropriate unit tests.
  • Pull request title or commits are in conventional commit format (e.g. feat: or fix:)

resolves #9582

If the client connection only gets closed, the next invocation of Connect
(via Gather loop) would try to invoke CloseSession on an already-closed
client.

It might be possible to get rid of the nil-check

if o.client != nil {

in Connect but further investigation is needed.

If the client connection only gets closed, the next invocation of `Connect`
(via `Gather` loop) would try to invoke `CloseSession` on an already-closed
client.

It might be possible to get rid of the nil-check in
```
if o.client != nil {
```

in `Connect` but further investigation is needed.
Copy link
Copy Markdown
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @milgner for this PR!

The code looks good to me. And it potentially saves out souls for the (seemingly) related #9551.

@srebhan srebhan self-assigned this Aug 4, 2021
@srebhan srebhan added area/opcua fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. labels Aug 4, 2021
Copy link
Copy Markdown
Contributor

@sspaink sspaink left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@sspaink sspaink merged commit 7def7e7 into influxdata:master Aug 10, 2021
phemmer added a commit to phemmer/telegraf that referenced this pull request Aug 13, 2021
* origin/master: (183 commits)
  fix: CrateDB replace dots in tag keys with underscores (influxdata#9566)
  feat: Pull metrics from multiple AWS CloudWatch namespaces (influxdata#9386)
  fix: improve Clickhouse corner cases for empty recordset in aggregation queries, fix dictionaries behavior (influxdata#9401)
  fix(opcua): clean client on disconnect so that connect works cleanly (influxdata#9583)
  fix: Refactor ec2 init for config-api (influxdata#9576)
  fix: sort logs by timestamp before writing to Loki (influxdata#9571)
  fix: muting tests for udp_listener (influxdata#9578)
  fix: Do not return on disconnect to avoid breaking reconnect (influxdata#9524)
  fix: Fixing k8s nodes and pods parsing error (influxdata#9581)
  feat: OpenTelemetry output plugin (influxdata#9228)
  feat: Support AWS Web Identity Provider (influxdata#9411)
  fix: upgraded sensu/go to v2.9.0 (influxdata#9577)
  fix: Normalize unix socket path (influxdata#9554)
  docs: fix aws ec2 readme inconsistency (influxdata#9567)
  feat: Modbus Rtu over tcp enhancement (influxdata#9570)
  docs: information on new conventional commit format (influxdata#9573)
  docs: Add logo (influxdata#9574)
  docs: Adding links to net_irtt and dht_sensor external plugins (influxdata#9569)
  Upgrade hashicorp/consul/api to 1.9.1 (influxdata#9565)
  Update vmware/govmomi to v0.26.0 (influxdata#9552)
  Do not skip good quality nodes after a bad quality node is encountered (influxdata#9550)
  fix test so it hits a fake service (influxdata#9564)
  Update changelog
  Fix procstat plugin README to match sample config (influxdata#9553)
  Fix metrics reported as written but not actually written  (influxdata#9526)
  Prevent segfault in persistent volume claims (influxdata#9549)
  Update procstat to support cgroup globs & include systemd unit children (Copy of influxdata#7890) (influxdata#9488)
  Fix attempt to connect to an empty list of servers. (influxdata#9503)
  Fix handling bool in sql input plugin (influxdata#9540)
  Suricata alerts (influxdata#9322)
  Linter fixes for plugins/inputs/[fg]* (influxdata#9387)
  For Prometheus Input add ability to query Consul Service catalog (influxdata#5464)
  Support Landing page on Prometheus landing page (influxdata#8641)
  [Docs] Clarify tagging behavior (influxdata#9461)
  Change the timeout from all queries to per query (influxdata#9471)
  Attach the pod labels to the `kubernetes_pod_volume` & `kubernetes_pod_network` metrics. (influxdata#9438)
  feat(http_listener_v2): allows multiple paths and add path_tag (influxdata#9529)
  Bug Fix Snmp empty metric name (influxdata#9519)
  Worktable workfile stats (influxdata#8587)
  Update Go to v1.16.6 (influxdata#9542)
  ...
reimda pushed a commit that referenced this pull request Aug 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/opcua fix pr to fix corresponding bug ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OPCUA Input: client should be set to nil after closing

4 participants