Merge pull request mendersoftware#983 from kacf/zombie_fix

Fix: Don't accumulate zombies when identity/inventory parsing fails.

app/process_leaking_test.go

@@ -0,0 +1,24 @@
+// Copyright 2022 Northern.tech AS
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package app
+
+import (
+ "testing"
+
+ stest "github.com/mendersoftware/mender/system/testing"
+)
+
+func TestZombieProcessLeaking(t *testing.T) {
+ stest.TestZombieProcessLeaking(t)
+}

app/proxy/process_leaking_test.go

@@ -0,0 +1,24 @@
+// Copyright 2022 Northern.tech AS
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package proxy
+
+import (
+ "testing"
+
+ stest "github.com/mendersoftware/mender/system/testing"
+)
+
+func TestZombieProcessLeaking(t *testing.T) {
+ stest.TestZombieProcessLeaking(t)
+}

app/updatecontrolmap/process_leaking_test.go

@@ -0,0 +1,24 @@
+// Copyright 2022 Northern.tech AS
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package updatecontrolmap
+
+import (
+ "testing"
+
+ stest "github.com/mendersoftware/mender/system/testing"
+)
+
+func TestZombieProcessLeaking(t *testing.T) {
+ stest.TestZombieProcessLeaking(t)
+}

cli/process_leaking_test.go

@@ -0,0 +1,24 @@
+// Copyright 2022 Northern.tech AS
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package cli
+
+import (
+ "testing"
+
+ stest "github.com/mendersoftware/mender/system/testing"
+)
+
+func TestZombieProcessLeaking(t *testing.T) {
+ stest.TestZombieProcessLeaking(t)
+}

client/process_leaking_test.go

@@ -0,0 +1,24 @@
+// Copyright 2022 Northern.tech AS
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package client
+
+import (
+ "testing"
+
+ stest "github.com/mendersoftware/mender/system/testing"
+)
+
+func TestZombieProcessLeaking(t *testing.T) {
+ stest.TestZombieProcessLeaking(t)
+}

conf/process_leaking_test.go

@@ -0,0 +1,24 @@
+// Copyright 2022 Northern.tech AS
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package conf
+
+import (
+ "testing"
+
+ stest "github.com/mendersoftware/mender/system/testing"
+)
+
+func TestZombieProcessLeaking(t *testing.T) {
+ stest.TestZombieProcessLeaking(t)
+}

datastore/process_leaking_test.go

@@ -0,0 +1,24 @@
+// Copyright 2022 Northern.tech AS
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package datastore
+
+import (
+ "testing"
+
+ stest "github.com/mendersoftware/mender/system/testing"
+)
+
+func TestZombieProcessLeaking(t *testing.T) {
+ stest.TestZombieProcessLeaking(t)
+}

dbus/process_leaking_test.go

@@ -0,0 +1,24 @@
+// Copyright 2022 Northern.tech AS
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package dbus
+
+import (
+ "testing"
+
+ stest "github.com/mendersoftware/mender/system/testing"
+)
+
+func TestZombieProcessLeaking(t *testing.T) {
+ stest.TestZombieProcessLeaking(t)
+}

device/identity_data.go

@@ -1,4 +1,4 @@
-// Copyright 2021 Northern.tech AS
+// Copyright 2022 Northern.tech AS
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -69,14 +69,19 @@ func (id IdentityDataRunner) Get() (string, error) {
  }
 
  p := utils.KeyValParser{}
- if err := p.Parse(out); err != nil {
- return "", errors.Wrapf(err, "failed to parse identity data")
- }
+ parseErr := p.Parse(out)
 
  if err := cmd.Wait(); err != nil {
+ if parseErr != nil {
+ err = errors.Wrapf(parseErr, "failed to parse identity data")
+ }
  return "", errors.Wrapf(err, "wait for helper failed")
  }
 
+ if parseErr != nil {
+ return "", errors.Wrapf(parseErr, "failed to parse identity data")
+ }
+
  collected := p.Collect()
  if len(collected) == 0 {
  return "", errors.New("no identity data colleted")

device/process_leaking_test.go

@@ -0,0 +1,24 @@
+// Copyright 2022 Northern.tech AS
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package device
+
+import (
+ "testing"
+
+ stest "github.com/mendersoftware/mender/system/testing"
+)
+
+func TestZombieProcessLeaking(t *testing.T) {
+ stest.TestZombieProcessLeaking(t)
+}

installer/bootenv.go

@@ -236,8 +236,9 @@ func (e *UBootEnv) getEnvironmentVariable(args []string) (BootVars, error) {
  var cmd *system.Cmd
  var cmdReader io.Reader
  var err error
+ var executable string
  found := false
- for _, executable := range e.getCommand {
+ for _, executable = range e.getCommand {
  cmd = e.Command(executable, args...)
 
  cmdReader, err = cmd.StdoutPipe()
@@ -280,8 +281,18 @@ func (e *UBootEnv) getEnvironmentVariable(args []string) (BootVars, error) {
 
  //we have some malformed data or Warning/Error
  if len(splited_line) != 2 {
+ // Empty scanner to avoid deadlock.
+ for scanner.Scan() {
+ }
+ err = cmd.Wait()
+ message := "Invalid U-Boot variable or error: " + scanner.Text()
+ if err != nil {
+ err = errors.Wrap(err, message)
+ } else {
+ err = errors.New(message)
+ }
  log.Error("U-Boot variable malformed or error occurred")
- return nil, errors.New("Invalid U-Boot variable or error: " + scanner.Text())
+ return nil, err
  }
 
  env_variables[splited_line[0]] = splited_line[1]

installer/process_leaking_test.go

@@ -0,0 +1,24 @@
+// Copyright 2022 Northern.tech AS
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package installer
+
+import (
+ "testing"
+
+ stest "github.com/mendersoftware/mender/system/testing"
+)
+
+func TestZombieProcessLeaking(t *testing.T) {
+ stest.TestZombieProcessLeaking(t)
+}

inventory/inventory_data.go

@@ -1,4 +1,4 @@
-// Copyright 2021 Northern.tech AS
+// Copyright 2022 Northern.tech AS
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -89,16 +89,18 @@ func (id *InventoryDataRunner) Get() (client.InventoryData, error) {
  }
 
  p := utils.KeyValParser{}
- if err := p.Parse(out); err != nil {
-  log.Warnf("Inventory tool %s returned unparsable output: %v", t, err)
- continue
+ var parseErr error
+ if parseErr = p.Parse(out); parseErr != nil {
+ log.Warnf("Inventory tool %s returned unparsable output: %v", t, parseErr)
  }
 
  if err := cmd.Wait(); err != nil {
  log.Warnf("Inventory tool %s wait failed: %v", t, err)
  }
 
- idec.AppendFromRaw(p.Collect())
+ if parseErr == nil {
+ idec.AppendFromRaw(p.Collect())
+ }
  }
  return idec.GetInventoryData(), nil
 }

inventory/inventory_data_test.go

@@ -1,4 +1,4 @@
-// Copyright 2020 Northern.tech AS
+// Copyright 2022 Northern.tech AS
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -14,10 +14,14 @@
 package inventory
 
 import (
+ "io/ioutil"
+ "os"
+ "path"
  "testing"
 
  "github.com/mendersoftware/mender/client"
  "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
 )
 
 func TestInventoryDataDecoder(t *testing.T) {
@@ -64,3 +68,22 @@ func TestInventoryDataDecoder(t *testing.T) {
  Name: "bar",
  Value: "zen"})
 }
+
+func TestInventoryDataParseError(t *testing.T) {
+ tmpDir, err := ioutil.TempDir("", "")
+ require.NoError(t, err)
+ defer os.RemoveAll(tmpDir)
+
+ fd, err := os.OpenFile(path.Join(tmpDir, "mender-inventory-test"),
+ os.O_CREATE|os.O_WRONLY, 0755)
+ require.NoError(t, err)
+ fd.Write([]byte("#!/bin/sh\necho bogus\n"))
+ fd.Close()
+
+ inventory := NewInventoryDataRunner(tmpDir)
+ data, err := inventory.Get()
+ // Does not return individial errors, only logging, but should result in
+ // empty inventory data.
+ assert.NoError(t, err)
+ assert.Equal(t, 0, len(data))
+}