thiagoralves / OpenPLC_v3

OpenPLC Runtime version 3
GNU General Public License v3.0
1.08k stars 438 forks source link

[development] Fix services not starting and introduce more precise exception handling when connecting to runtime and improvements to Docker setup. #159

Closed vembacher closed 2 years ago

vembacher commented 2 years ago

Issue

refer to #158. This pull request should fix this issue. Note: this also affected many other services like enip, dnp3s, etc.

Cause

This example should explain the issue:

    def start_modbus(self, port_num):
        if (self.status() == "Running"):
            try:
                s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
                s.connect(('localhost', 43628))
               # the next line causes a TypeError because we can't concatenate a string to bytes 
                s.send(b'start_modbusslave(' + str(port_num) + ')\n') # <------  TypeError raised here
                data = s.recv(1000)
                s.close()
            # This part catches the TypeErrror, because it catches every exception, leaving us clueless.
            except:                                                                            
                print("Error connecting to OpenPLC runtime")

Fix

It is fixed by making sure we're only concatenating bytes to bytes and we're introducing more precise exception handling so we are not blind to unexpectedly raised exceptions.

    def start_modbus(self, port_num):
        if (self.status() == "Running"):
            try:
                s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
                s.connect(('localhost', 43628))
                # use only bytes
                s.send(b'start_modbusslave(' + (b'%d' % port_num) + b')\n') 
                data = s.recv(1000)
                s.close()
            # connecting to an unopened port on localhost should only cause a ConnectionRefusedError
            except ConnectionRefusedError: 
                print("Error connecting to OpenPLC runtime")
vembacher commented 2 years ago

The pull-request has expanded a bit because I wasn't aware it always points to the most recent commit of the branch instead of the most recent when creating the pull-request. So I will explain all the other changes here:

Further Changes

The capabilities should cover all the privileges the runtime needs. (Source)

SYS_NICE

- Lower the process nice value (nice(2), setpriority(2)) and change the nice value for arbitrary processes; - set real-time scheduling policies for calling process, and set scheduling policies and priorities for arbitrary processes (sched_setscheduler(2), sched_setparam(2), sched_setattr(2)); - set CPU affinity for arbitrary processes (sched_setaffinity(2)); - set I/O scheduling class and priority for arbitrary processes (ioprio_set(2)); - apply migrate_pages(2) to arbitrary processes and allow processes to be migrated to arbitrary nodes; - apply move_pages(2) to arbitrary processes; - use the MPOL_MF_MOVE_ALL flag with mbind(2) and move_pages(2).

CAP_IPC_LOCK

- Lock memory (mlock(2), mlockall(2), mmap(2), shmctl(2)); - Allocate memory using huge pages (memfd_create(2), mmap(2), shmctl(2)).