When running with a single CPU, there is a deadlock when collecting results from
the workers. The deadlock was observed on Python 3.6.5+). See the backtraces
below for more details. Temporary workaround is to bump the workers count to at
least 2.
(gdb) py-list
297 self._waiters.append(waiter)
298 saved_state = self._release_save()
299 gotit = False
300 try: # restore state no matter what (e.g., KeyboardInterrupt)
301 if timeout is None:
>302 waiter.acquire()
303 gotit = True
304 else:
305 if timeout > 0:
306 gotit = waiter.acquire(True, timeout)
307 else:
(gdb) py-bt
Traceback (most recent call first):
File "/usr/lib/python3.8/threading.py", line 302, in wait
waiter.acquire()
File "/usr/lib/python3.8/concurrent/futures/_base.py", line 434, in result
self._condition.wait(timeout)
File "/usr/lib/python3.8/concurrent/futures/_base.py", line 611, in result_iterator
yield fs.pop().result()
File "./spread-shellcheck", line 208, in checkpaths
File "./spread-shellcheck", line 236, in main
File "./spread-shellcheck", line 542, in <module>
(gdb) info threads
Id Target Id Frame
* 1 Thread 0x7f570954e740 (LWP 22590) "python3" 0x00007f570970a3f4 in do_futex_wait.constprop () from /lib/x86_64-linux-gnu/libpthread.so.0
2 Thread 0x7f5708c62700 (LWP 22591) "python3" 0x00007f570970a3f4 in do_futex_wait.constprop () from /lib/x86_64-linux-gnu/libpthread.so.0
(gdb) thread 2
[Switching to thread 2 (Thread 0x7f5708c62700 (LWP 22591))]
(gdb) py-bt
Traceback (most recent call first):
File "/usr/lib/python3.8/threading.py", line 302, in wait
waiter.acquire()
File "/usr/lib/python3.8/concurrent/futures/_base.py", line 434, in result
self._condition.wait(timeout)
File "./spread-shellcheck", line 176, in checkfile
File "./spread-shellcheck", line 198, in check1path
File "/usr/lib/python3.8/concurrent/futures/thread.py", line 57, in run
result = self.fn(*self.args, **self.kwargs)
File "/usr/lib/python3.8/concurrent/futures/thread.py", line 80, in _worker
work_item.run()
File "/usr/lib/python3.8/threading.py", line 870, in run
self._target(*self._args, **self._kwargs)
File "/usr/lib/python3.8/threading.py", line 932, in _bootstrap_inner
self.run()
File "/usr/lib/python3.8/threading.py", line 890, in _bootstrap
self._bootstrap_inner()
(gdb)
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
spread-shellcheck imposes a timeout on each shellcheck instance that
runs, so that it automatically stops and fails after 10 seconds. This
was done long ago, IIRC, to allow shellcheck to not run forever on
malformed input.
With the increased concurrency, on commonly used machines using
symmetric multi-threading, the OS sees virtual CPU cores that are
suitable for concurrency at a thread level, not at a process level.
The extra system load from the overall parallelism is now pushing the
execution of the slowest shellcheck, at least on a Intel Haswell-era
mobile core i7, past the 10 second limit.
Since the limit is pretty much entirely arbitrary, increase it to 30
seconds.
Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
Before it was a nested function, which prevented the usage of
ProcessPoolExecutor, as those cannot be picked.
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Attempt to speed up the spread-shellcheck execution a bit, by processing the
paths passed in the arguments in parallel. In particular, the run-checks command
executes: `spread-shellcheck spread.yaml tests`. The old code would firs process
spread.yaml, and then proceed to walk tests and process all files in
parallel (limited to by the workers count). I
t also happens that spread.yaml is composed of many longish sections and the
processing of a complete file takes longer than for a typical test.
The patch makes all the locations passed in the command line be processed in
parallel by one executors pool.
A bit of data, running `spread-shellcheck spread.yaml tests` on my machine:
- baseline: 75s
- with the patch: 59s
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Fix the warning when loading PyYAML:
./spread-shellcheck:140: YAMLLoadWarning: calling yaml.load() without Loader=...
is deprecated, as the default Loader is unsafe.
Please read https://msg.pyyaml.org/load for full details.
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Spread support test variants where the environment section can be used
to convey specific variant information. In terms of YAML it looks like
this:
environment:
key/A: variant-a
key/B: variant-b
execute: |
echo "$key"
Here key/A and key/B define the value of the environment variable "key"
and depending on the execution variant, the values will be "variant-a"
and "variant-b" respectively.
Currently spread-shellcheck would complain that the variable "$key" is
unset. With this patch it will know the definition of some of the
variant values. This is suboptimal but allows spread-shellcheck to not
complain about something that is clearly not broken.
As a future extension the checker could re-check the script with each
variant, carrying the exact values each time, in the hope that constant
propagation in shellcheck can make use of that information.
Signed-off-by: Zygmunt Krynicki <me@zygoon.pl>
When shellcheck identifies erorrs in many files, the error log will be
interleaved as the logging is happening from inside of the helper ran by a
thread pool executor. Tweak the code so that the helpers return the error
instead of doing the logging directly.
Minor logging cleanup.
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
Current spread-shellcheck does things serially. The task is of a class
called "embarrassingly parallel", so I did the obvious thing:
Before:
$ time ./spread-shellcheck spread.yaml tests
real 1m27.587s
user 0m53.500s
sys 0m15.368s
After:
$ time ./spread-shellcheck spread.yaml tests
real 0m13.977s
user 0m56.736s
sys 0m11.536s
One thing to improve would be to add a lock and hold it around any
call to logging (especially as we do more than one call to logging in
a row and expect them to be adjacent in the output). In practice, not
really a problem right now.
Spread shell snippets are executed under set -e shell, passing this information
to shellcheck makes some weird issues such as SC2164 go away.
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
In an attempt to start enforcing spread-shellcheck on newly added files, add
support for whitelisting of files that can fail validation. This way, we'll be
able to list all the known invalid files and new files will be automatically
included.
Signed-off-by: Maciej Borzecki <maciej.zenon.borzecki@canonical.com>
