[bug] SSH connection leaked when update_config() raises an exception

[mn-ram]

● Describe the bug

In DeviceConnection.update_config(), the SSH connection is only closed
in the else block, which means it is never disconnected when an
exception occurs during a config push. Over time this leaks SSH file
descriptors and can crash the controller process with:

OSError: [Errno 24] Too many open files

---

● Steps To Reproduce

1. Set up a device with an SSH DeviceConnection
2. Make the config push fail intentionally (e.g. wrong credentials, unreachable device, or network drop mid-transfer)
3. Observe that no disconnect() is called — the SSH session stays open
4. Repeat for multiple devices; file descriptor count keeps growing

---

● Root cause (file + line)

File: openwisp_controller/connection/base/models.py
Lines: 365–373

def update_config(self):
    self.connect()
    if self.is_working:
        try:
            self.connector_instance.update_config()
        except Exception as e:
            logger.exception(e)
        else:
            self.disconnect()  # ← only runs when NO exception is raised

The else block in a try/except/else only executes when the try
block completes without raising. Any exception causes disconnect()
to be skipped entirely.

---

● Expected behavior

The SSH connection should always be closed after update_config()
completes — whether it succeeded or failed — to prevent file descriptor
leaks on production controllers.

---

● Proposed fix

Change else to finally:

def update_config(self):
    self.connect()
    if self.is_working:
        try:
            self.connector_instance.update_config()
        except Exception as e:
            logger.exception(e)
        finally:
            self.disconnect()  # ← runs always, success OR failure

---

● Screenshots

Not applicable (code-level bug).

---

● System Information

• OS: Ubuntu 24.04 LTS
• Python Version: Python 3.10+
• Django Version: Django 4.2+
• Browser and Browser Version (if applicable): Not applicable

[tomsebastian10]

Hi, I’d like to work on this issue.

The proposed fix using a finally block makes sense to ensure that the SSH connection is always closed. I also plan to add a test case to verify that disconnect() is called even when an exception occurs during update_config().
Please let me know if this approach aligns with your expectations.

[mn-ram]

Hi @tomsebastian10 , thanks for your interest in this issue!

I actually opened this issue and I’m currently working on fixing it. I’ll be submitting a PR soon.

If I get stuck or need help, I’ll definitely let you know. Appreciate your willingness to contribute! 🙂