fix: tedge mqtt ignoring ctrl-c when connection fails

[didier-wenzek]

Proposed changes

As rumqttc doesn't trigger a client disconnect when the connection is not established,
one has to combine a client disconnect with the use of an AtomicBool to break the event loop
if for some reason the connection is not established when the user hits ctrl-c.

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
• Documentation Update (if none of the other choices apply)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#2949

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA (in all commits with git commit -s)
• I ran cargo fmt as mentioned in CODING_GUIDELINES
• I used cargo clippy as mentioned in CODING_GUIDELINES
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

Further comments

[codecov]

Codecov Report

Attention: Patch coverage is 44.44444% with 5 lines in your changes missing coverage. Please review.

Project coverage is 78.2%. Comparing base (5826ae2) to head (ed0b7fd).
Report is 10 commits behind head on main.

Additional details and impacted files

Files	Coverage Δ
crates/core/tedge/src/cli/mqtt/mod.rs	66.6% <80.0%> (-2.6%)	⬇️
crates/core/tedge/src/cli/mqtt/subscribe.rs	0.0% <0.0%> (ø)

... and 11 files with indirect coverage changes

[github-actions]

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %	⏱️ Duration
453	0	3	453	100	1h1m13.030747999s

[albinsuresh]

I was initially wondering why the same is not done for publish as well, but then noticed that the publisher already breaks out of the loop on any connection error. I'm assuming that the behaviour is different here (not breaking out of the loop on any connection error) to handle reconnection cases as well, esp for long running subscribers.

[didier-wenzek]

Yes, the point is to handle long running subscribers ignoring connection blips unless the user requests to break.

[albinsuresh]

Since the performance of these utilities is not at all a concern, but immediate response to the Ctrl + C command is desired, isn't it better to go with a stronger ordering guarantee using Ordering::Acquire and Ordering::Release so that the program closes as soon as Ctrl + C is pressed?

[didier-wenzek]

Yeah. A user will not notice the difference. Besides, the main delay is introduced by the event loop. The break will occur only after a connection error is reported.

[reubenmiller]

The delay (of up to 1 second) between pressing ctrl+c is on there when the MQTT broker is not reachable, so I don't think the user will run into it very often, so it's ok from UX perspective.

[reubenmiller]

Approved