Shallow copy of container object in Containers(), DefinedContainers() and ActiveContainers()

[aftab-a]

We are writing a daemon to monitor health of containers running on our server farm. The daemon was hitting panic at random times. Essentially we are doing the following in a loop :- func ListLXCInstances() {     c := lxc.Containers()     for i := range c {         //Push container name and state to a central server.     } }

Code panics when we were trying to read either state or name from the container object using Name() or State() function. On digging around I found Release function was getting called. Following line seems to be issue here :-

for _, v := range ContainerNames(lxcpath...) {
        if container, err := NewContainer(v, lxcpath...); err == nil {
            containers = append(containers, *container)
        }
    }

Here we are making object copy but we are not incrementing reference counter on the underlying C struct (container *C.struct_lxc_container). As we move out of this function, object created by NewContainer() can be destroyed if gc is run. If gc is run at this time, underlying object gets destroyed, while the container objects returned by Containers() function points to the free memory location. This could cause panic when accessed. Following code can easily highlight the issue :- func ListLXCInstances() {     c := lxc.Containers()     runtime.GC() // For a GC.     for i := range c {         fmt.Println(c[i].Name(), c[i].State())     } }

Here Println() will print garbage values or it could cause panic. Please let me know if my analysis is wrong or if I am using the api correctly.

[caglar10ur]

Have you tried calling Acquire (https://github.com/lxc/go-lxc/blob/de2c8bfd65a78752d6a70b4ad99114c6969363b0/lxc-binding.go#L51) ?

[caglar10ur]

We could also do following (not tested)

diff --git a/lxc-binding.go b/lxc-binding.go
index a45648f..d01d3c0 100644
--- a/lxc-binding.go
+++ b/lxc-binding.go
@@ -116,12 +116,12 @@ func ContainerNames(lxcpath ...string) []string {

 // Containers returns the defined and active containers on the system. Only
 // containers that could retrieved successfully are returned.
-func Containers(lxcpath ...string) []Container {
-       var containers []Container
+func Containers(lxcpath ...string) []*Container {
+       var containers []*Container

        for _, v := range ContainerNames(lxcpath...) {
                if container, err := NewContainer(v, lxcpath...); err == nil {
-                       containers = append(containers, *container)
+                       containers = append(containers, container)
                }
        }

@@ -151,12 +151,12 @@ func DefinedContainerNames(lxcpath ...string) []string {

 // DefinedContainers returns the defined containers on the system.  Only
 // containers that could retrieved successfully are returned.
-func DefinedContainers(lxcpath ...string) []Container {
-       var containers []Container
+func DefinedContainers(lxcpath ...string) []*Container {
+       var containers []*Container

        for _, v := range DefinedContainerNames(lxcpath...) {
                if container, err := NewContainer(v, lxcpath...); err == nil {
-                       containers = append(containers, *container)
+                       containers = append(containers, container)
                }
        }

@@ -186,18 +186,19 @@ func ActiveContainerNames(lxcpath ...string) []string {

 // ActiveContainers returns the active containers on the system. Only
 // containers that could retrieved successfully are returned.
-func ActiveContainers(lxcpath ...string) []Container {
-       var containers []Container
+func ActiveContainers(lxcpath ...string) []*Container {
+       var containers []*Container

        for _, v := range ActiveContainerNames(lxcpath...) {
                if container, err := NewContainer(v, lxcpath...); err == nil {
-                       containers = append(containers, *container)
+                       containers = append(containers, container)
                }
        }

        return containers
 }

which also silences the go vet error (shallow copy copies the underlying mutex)

[caglar10ur]

Now that https://github.com/lxc/go-lxc/pull/86 is merged, I'm closing this