avocado.main(): avoid an infinite fork loop bomb [v2] #977

clebergnu · 2016-01-25T09:56:55Z

Because the current implementation of avocado.main() creates a job
and runs "sys.argv[0]" to implement standalone mode, it ends up
running itself over and over.

This simple proposed fix, prevents avocado.main() from running
itself again if called from itself. Since they are on different
processes, the mechanism chosen to do this is to set an environment
variable, that will be seen by the next process.

This adresses issue #961.

Signed-off-by: Cleber Rosa crosa@redhat.com

Changes from v1:

Add error message to STDERR, warning that main() will exit to avoid a loop.

Because the current implementation of avocado.main() creates a job and runs "sys.argv[0]" to implement standalone mode, it ends up running itself over and over. This simple proposed fix, prevents avocado.main() from running itself again if called from itself. Since they are on different processes, the mechanism chosen to do this is to set an environment variable, that will be seen by the next process. This adresses issue avocado-framework#961. Signed-off-by: Cleber Rosa <crosa@redhat.com>

apahim · 2016-01-25T10:55:39Z

Looks great.

ldoktor · 2016-01-25T11:01:43Z

Well actually even without the error code return it fails. What is more interesting is, that I get 2 different results when running broken test:

python a.py
...
START a.py
PARAMS (key=timeout, path=*, default=None) => None
Running 'a.py'

Reproduced traceback from: /home/medic/Work/Projekty/avocado/avocado/avocado/core/test.py:372
Traceback (most recent call last):
  File "/home/medic/Work/Projekty/avocado/avocado/avocado/core/test.py", line 591, in test
    env=test_params)
  File "/home/medic/Work/Projekty/avocado/avocado/avocado/utils/process.py", line 962, in run
    cmd_result = sp.run(timeout=timeout)
  File "/home/medic/Work/Projekty/avocado/avocado/avocado/utils/process.py", line 500, in run
    self._init_subprocess()
  File "/home/medic/Work/Projekty/avocado/avocado/avocado/utils/process.py", line 290, in _init_subprocess
    raise exc
OSError: File 'a.py' not found

Traceback (most recent call last):

  File "/home/medic/Work/Projekty/avocado/avocado/avocado/core/test.py", line 427, in _run_avocado
    raise test_exception

OSError: File 'a.py' not found

ERROR a.py -> OSError: File 'a.py' not found

Test results available in /tmp/avocado_avocado.core.jobxxkohH

vs.

python ./a.py
...
START ./a.py
PARAMS (key=timeout, path=*, default=None) => None
Running './a.py'
[stderr] AVOCADO_STANDALONE_IN_MAIN environment variable found. This means that this code is being called recursively. Exiting to avoid an infinite fork loop.
AVOCADO_STANDALONE_IN_MAIN environment variable found. This means that this code is being called recursively. Exiting to avoid an infinite fork loop.
[stderr] Traceback (most recent call last):
Traceback (most recent call last):
[stderr]   File "./a.py", line 13, in <module>
  File "./a.py", line 13, in <module>
[stderr]     main()
    main()
[stderr] TypeError: __init__() should return None, not 'int'
TypeError: __init__() should return None, not 'int'
Exit status: 1
Duration: 0.139937877655

Reproduced traceback from: /home/medic/Work/Projekty/avocado/avocado/avocado/core/test.py:372
Traceback (most recent call last):
  File "/home/medic/Work/Projekty/avocado/avocado/avocado/core/test.py", line 596, in test
    raise exceptions.TestFail(details)
TestFail: Command './a.py' failed (rc=1)

FAIL ./a.py -> TestFail: Command './a.py' failed (rc=1)

Test results available in /tmp/avocado_avocado.core.jobvGzceC

I like the second, but the first one is really confusing and even worst avocado run a.py behaves the same. Anyway I guess it's not related to this fix, so I agree with it, we still need to investigate the a.py vs. ./a.py...

clebergnu · 2016-01-25T11:07:32Z

Strange, what's your a.py like?

This is my test code:

#!/usr/bin/env python
from avocado import main

if __name__ == '__main__':
    main()

And then running it:

$ ./t.py 
Command line: ./t.py

Avocado version: 0.32.0

Config files read (in order):
/home/cleber/src/avocado/avocado/etc/avocado/avocado.conf
/home/cleber/src/avocado/avocado/etc/avocado/conf.d/gdb.conf

Avocado config:
Section.Key                             Value
datadir.paths.base_dir                  /usr/share/avocado
datadir.paths.test_dir                  /usr/share/avocado/tests
datadir.paths.data_dir                  /usr/share/avocado/data
datadir.paths.logs_dir                  ~/avocado/job-results
sysinfo.collect.enabled                 True
sysinfo.collect.installed_packages      False
sysinfo.collect.profiler                False
sysinfo.collectibles.commands           /etc/avocado/sysinfo/commands
sysinfo.collectibles.files              /etc/avocado/sysinfo/files
sysinfo.collectibles.profilers          /etc/avocado/sysinfo/profilers
runner.output.colored                   True
runner.output.utf8                      
runner.behavior.keep_tmp_files          False
job.output.loglevel                     debug
restclient.connection.hostname          localhost
restclient.connection.port              9405
restclient.connection.username          
restclient.connection.password          
plugins.skip_broken_plugin_notification []
plugins.loaders                         ['file', '@DEFAULT']
gdb.paths.gdb                           /usr/bin/gdb
gdb.paths.gdbserver                     /usr/bin/gdbserver

Avocado Data Directories:

Avocado replaces config dirs that can't be accessed
with sensible defaults. Please edit your local config
file to customize values

base     /home/cleber/avocado
tests    /home/cleber/src/avocado/avocado/examples/tests
data     /home/cleber/avocado/data
logs     /home/cleber/avocado/job-results

Temporary dir: /tmp/avocado_75OCMy

Variant 1:    /

Job ID: 28257052eede46b38be927bec048d4fada3e11f4

Commands configured by file: /etc/avocado/sysinfo/commands
Files configured by file: /etc/avocado/sysinfo/files
Profilers configured by file: /etc/avocado/sysinfo/profilers
Profilers declared: []
Profiler disabled: no profiler commands configured
START ./t.py
PARAMS (key=timeout, path=*, default=None) => None
Running './t.py'
[stderr] AVOCADO_STANDALONE_IN_MAIN environment variable found. This means that this code is being called recursively. Exiting to avoid an infinite fork loop.
AVOCADO_STANDALONE_IN_MAIN environment variable found. This means that this code is being called recursively. Exiting to avoid an infinite fork loop.
Exit status: 0
Duration: 0.14125418663
Not logging /var/log/messages (lack of permissions)
PASS ./t.py

Test results available in /tmp/avocado_avocado.core.jobKDUkbG

It PASSes.

ldoktor · 2016-01-25T11:25:20Z

OK, I accidentally used to version with return -1... Running: ./b.py and python ./b.py passes, but python b.py fails with the OSException mentioned earlier.

Anyway I think the OSException is not related to your fix so we should deal with it separately. What I dislike is the PASS, because in case of typo you get pass:

a.py:

#!/usr/bin/env python

from avocado import main
from avocado import Test

'''
class Passtest(Test):
    def tst(self):
        pass
'''

if __name__ == "__main__":
    main()

clebergnu · 2016-01-25T11:28:27Z

@ldoktor btw, the different behavior regarding avocado run a.py and avocado run ./a.py is probably related/can be "fixed" with:

diff --git a/avocado/core/test.py b/avocado/core/test.py
index 15db099..d669b29 100644
--- a/avocado/core/test.py
+++ b/avocado/core/test.py
@@ -559,7 +559,7 @@ class SimpleTest(Test):
     def __init__(self, name, params=None, base_logdir=None, tag=None, job=None):
         super(SimpleTest, self).__init__(name=name, base_logdir=base_logdir,
                                          params=params, tag=tag, job=job)
-        self.path = name
+        self.path = os.path.abspath(name)
         basedir = os.path.dirname(self.path)
         basename = os.path.basename(self.path)
         datadirname = basename + '.data'

lmr · 2016-01-25T16:32:43Z

@clebergnu I think the fix has the side effect of showing all standalone tests with their full paths in the avocado UI.

ldoktor · 2016-01-25T17:08:29Z

Well the fix also might influence the remote runner, but I haven tested it... anyway lets just create a BUG card and deal with it separately...

lmr · 2016-01-25T17:11:32Z

Thinking about the problem @ldoktor found out, there's the possibility that somewhere in the code we're changing the current working directory and not resetting it. I'd look for suspicious os.chdir in the code.

lmr · 2016-01-25T17:16:24Z

I was git grepping and found out:

avocado/plugins/replay.py:

    # Use the original directory to discover test urls properly
    pwd = replay.retrieve_pwd(resultsdir)
    if pwd is not None:
        if os.path.exists(pwd):
            os.chdir(pwd)

I still need to see if this gets executed in normal paths.

apahim · 2016-01-25T17:20:43Z

This is used only if --replay option is in place::

 94     def run(self, args):
 95         if getattr(args, 'replay_jobid', None) is None:
 96             return

lmr · 2016-01-25T17:23:54Z

@apahim yep, my first guess was dead on the water - Apparently the cwd is correct, sorry about that.

I'll spend a few more minutes trying to figure out the bug.

lmr · 2016-01-25T19:02:16Z

No dice. There's something I'm missing here. The only fix that actually worked was the one proposed by @clebergnu (using abspath). And by the way, it does not affect the display of the file path in the avocado UI:

avocado run examples/tests/output_check.sh
JOB ID     : fbe115834542d8bfdb1d5fdc8a5723b199da71f9
JOB LOG    : /home/lmr/avocado/job-results/job-2016-01-25T16.59-fbe1158/job.log
TESTS      : 1
 (1/1) examples/tests/output_check.sh: PASS (0.01 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0
JOB HTML   : /home/lmr/avocado/job-results/job-2016-01-25T16.59-fbe1158/html/results.html
TIME       : 0.01 s

I remember we had a discussion about this a long time ago, but maybe the assumptions all changed and using the abspath is fine. I don't have much extra time to work on this, so please help.

clebergnu · 2016-01-27T12:43:41Z

Closing in favor of #985

lmr added the in progress label Jan 25, 2016

clebergnu mentioned this pull request Jan 25, 2016

avocado.main(): avoid an infinite fork loop bomb #975

Closed

clebergnu changed the title ~~avocado.main(): avoid an infinite fork loop bomb~~ avocado.main(): avoid an infinite fork loop bomb [v2] Jan 25, 2016

clebergnu closed this Jan 27, 2016

lmr removed the in progress label Jan 27, 2016

clebergnu deleted the avoid_fork_bomb_v2 branch May 4, 2016 21:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

avocado.main(): avoid an infinite fork loop bomb [v2] #977

avocado.main(): avoid an infinite fork loop bomb [v2] #977

clebergnu commented Jan 25, 2016

apahim commented Jan 25, 2016

ldoktor commented Jan 25, 2016

clebergnu commented Jan 25, 2016

ldoktor commented Jan 25, 2016

clebergnu commented Jan 25, 2016

lmr commented Jan 25, 2016

ldoktor commented Jan 25, 2016

lmr commented Jan 25, 2016

lmr commented Jan 25, 2016

apahim commented Jan 25, 2016

lmr commented Jan 25, 2016

lmr commented Jan 25, 2016

clebergnu commented Jan 27, 2016

avocado.main(): avoid an infinite fork loop bomb [v2] #977

avocado.main(): avoid an infinite fork loop bomb [v2] #977

Conversation

clebergnu commented Jan 25, 2016

apahim commented Jan 25, 2016

ldoktor commented Jan 25, 2016

clebergnu commented Jan 25, 2016

ldoktor commented Jan 25, 2016

clebergnu commented Jan 25, 2016

lmr commented Jan 25, 2016

ldoktor commented Jan 25, 2016

lmr commented Jan 25, 2016

lmr commented Jan 25, 2016

apahim commented Jan 25, 2016

lmr commented Jan 25, 2016

lmr commented Jan 25, 2016

clebergnu commented Jan 27, 2016