konimarti / opc

OPC DA client in Golang for monitoring and analyzing process data based on Windows COM.
MIT License
237 stars 85 forks source link

Bug in opcConnectionImpl fix() #14

Closed Lynckx closed 5 years ago

Lynckx commented 5 years ago

In the conn.fix() loop the conn.AutomationItems.Close() method causes the AutomationItems to be nil, this will result in the following panic: panic: runtime error: invalid memory address or nil pointer dereference @ line 396 in the conn.Tags() method and later @ line 322 in the conn.AutomationItems.Close() method when the first tryconnect was not successfull and it loops back. Maybe add a check to check if conn.AutomationItems != nil in these functions?

Lynckx commented 5 years ago

I was able to fix the issue. I rewrote the fix() method in connection_windows.go like this:

//fix tries to reconnect if connection is lost by creating a new connection
//with AutomationObject and creating a new AutomationItems instance.
func (conn *opcConnectionImpl) fix() {
    var err error
    var tags []string
    if !conn.IsConnected() {
        for {
            if conn.AutomationItems != nil {
                tags = conn.Tags()
                conn.AutomationItems.Close()
            }
            conn.AutomationItems, err = conn.TryConnect(conn.Server, conn.Nodes)
            if err != nil {
                logger.Println(err)
                time.Sleep(100 * time.Millisecond)
                continue
            }

            if conn.Add(tags...) == nil {
                logger.Printf("Added %d tags", len(tags))
            }
            break
        }
    }
}
konimarti commented 5 years ago

Thanks @Lynckx. Looks good! Could you also add a test to catch this in the future? Could you then submit a pull request so that I can merge it?

Lynckx commented 5 years ago

Made the pull request #15