Sort of but not really, this commit fixes a special case in which
launching a --test-file of a file with at least two SavepointCases would
create a postgresql deadlock and it would be impossible to terminate the
Odoo process without sending a SIGKILL or waiting for the lock to
timeout.
This was introduced at #39368 and happens because of the way that
unittests unwraps suites, to keep it short, when it unwraps the custom
OdooSuite class internally, it ends up with a vanilla TestSuite with
which to run the different test cases, and since #39368 depends on the
overrides added to OdooSuite to function, the class cleanups are not
triggered at the end of a test class (rollback, cache cleanups, env
reset, registry reset, etc.).
The fix is to manually unwrap the suite of tests to keep OdooSuite as
the suite with which to call the tests, which was already done for
--test-enable (although for different reasons, --test-tags?) which is
why --test-enable didn't have any problems.
This commit also fixes a typo I found on the backport, which meant
classCleanups were not being executed if the setUpClass failed, but it
had no effect on classCleanups during tearDownClass.
Task-ID 2160398
Depends on #43135closesodoo/odoo#43296
X-original-commit: 7a5ded7d40afc29043d356b5dece0dbe1fbd5ab3
Signed-off-by: Adrian Torres (adt) <adt@odoo.com>
An http-port provided on the command line (may also have been an issue
for config files, didn't check) would not be taken in account anymore,
because `odoo.tests.common` would be imported during the import of
`odoo` itself (when loading odoo.service.server), itself importing
`odoo.tools.config` leading to a default configuration being set up.
* remove `odoo.tests.common.PORT`, `config['http_port']` should be
used always
* defer the import of odoo.tests.common by moving it inside
load_test_file
* stop generating default configs
closesodoo/odoo#43283
X-original-commit: 45871f498ea4cf3ada719692e69cd413883ab442
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
Before this commit nothing prevented high concurrency on a threaded http
server to consume too much resources, ending up failing requests either
because the OS is unable to spawn that many threads
(`RuntimeError: can't start new thread`), either because the Odoo db
connection pool is full (`PoolError: The Connection Pool Is Full`).
This commit adds the ODOO_MAX_HTTP_THREADS environment variable which
allows to limit the amount of concurrent socket connections accepted by
a threaded server, implicitly limiting the amount of concurrent threads
running for http requests handling.
Note that if a value has been provided to ODOO_MAX_HTTP_THREADS that cannot
be parsed as an integer, a value will be automatically set to half the
db connection pool size (which defaults to 64). This dynamic value is
chosen because while most requests will borrow only one cursor
concurrently, there are some exceptions where some controllers might
allocate two or more cursors.
closesodoo/odoo#42327
X-original-commit: d42a951369ee87b50e834f73cd2af5cf031d43f7
Signed-off-by: Christophe Simonis <chs@odoo.com>
glibc's malloc() uses arenas [1] in order to efficiently handle memory
allocation of multi-threaded applications. This allows better memory
allocation handling in case of multiple threads that would be using
malloc() concurrently [2].
Due to the python's GIL, this optimization have no effect on
multithreaded python programs. Unfortunately, a downside of creating one
arena per cpu core is the increase of virtual memory which Odoo is based
upon in order to limit the memory usage for threaded workers.
On 32bit systems the default size of an arena is 512K while on 64bit
systems it's 64M [3], hence a threaded worker will quickly reach it's
default memory soft limit upon concurrent requests. We therefore set the
maximum arenas allowed to 2 unless the MALLOC_ARENA_MAX env variable is
set.
This commit also brings the following changes:
- allow to disable the memory hard limit for all servers if the provided
value is 0 (instead of crashing)
- increase the log level for threaded server in case of limits reached
Note: Setting MALLOC_ARENA_MAX=0 allow to explicitely set the default
glibs's malloc() behaviour.
[1] https://sourceware.org/glibc/wiki/MallocInternals#Arenas_and_Heaps
[2] https://www.gnu.org/software/libc/manual/html_node/The-GNU-Allocator.html
[3] https://sourceware.org/git/?p=glibc.git;a=blob;f=malloc/malloc.c;h=00ce48c;hb=0a8262a#l862closesodoo/odoo#42323
X-original-commit: 85fe2c6e60f7f1f6ea72cb55e85f85d420ce6616
Signed-off-by: Christophe Simonis <chs@odoo.com>
A deadlock can occur between threads when concurrent requests
acquire the registry lock and conflicting database-level locks
in different orders. The database won't be able to detect and
break the deadlock because it involves an external, Python-level
lock. This situation is more likely to occur during module
installations [1].
If the server is started with the `limit_time_real` option,
it should be able to abort the deadlocked requests after the
timeout, and restart. However that could not work because
the recovery initiated by `reload()` is blocked at the end
of the `stop()` method, as it cannot acquire the registry
lock either, necessary for `Registry.delete_all()`.
Since that deletion step is in fact not necessary, it can
be skipped, avoiding the deadlock entirely.
Indeed there's no real reason anymore to delete the DB's
registry upon shutdown. This was introduced for 7.0 by
b5daffc115, in order to perform
other cleanups (including cron agent threads). These other
cleanups are not necessary anymore, and when the stop()
method of the ThreadedServer completes, the next step is
either a restart of the whole process (via execve() through
_reexec()), or a full process exit. Keeping the registry in
memory for a few cycles until this happens makes no difference.
When such a deadlock occurs, it's always possible to manually
kill the server with 2 `kill` commands, or 1 `kill -9`.
~~~~~~~~~~~~~~~~~~~~~~
[1] Reproduction info:
The following deadlock was observed in Odoo threaded server mode:
1. incoming request spawns a new thread A
A starts a transaction and does a "SELECT ... FROM res_users ..."
getting an ACCESS SHARE lock on the table
2. incoming request spawns a new thread B
B is a request that calls `button_immediate_install`, that will
install new modules and alter the res_users table.
3. B takes and holds the registry lock and executes "ALTER TABLE
res_users ...", that waits to get the ACCESS EXCLUSIVE lock on the
table until A's transaction releases the ACCESS SHARE lock.
4. A continues code execution and reaches a .sudo() call, it tries to
create a new environment. The creation of the new environment
requires to wait for the registry's lock to be release but it's held
by B.
-> A waits for B's registry lock to be released
-> B waits for A's ACCESS SHARE lock to be released
-> Deadlock that can't be broken except by force-killing the server
closesodoo/odoo#40664
X-original-commit: 9e67525418b3b0a48a796044ef24b227946ceb8f
Signed-off-by: Olivier Dony (odo) <odo@openerp.com>
This commit partially backports bpo-24412, which allows the definition of
class cleanups (addClassCleanup) and module cleanups (omitted),
similar to instance cleanups (addCleanup).
This is useful for tests that override unittest's setUpClass and
could crash during its execution: If this happens, it is possible that a
bunch of crap is left in the database or even worse, the cursor becomes
completely fucked; Thanks to the addClassCleanup, we can undo the damage
done by the setUpClass.
Another benefit is that it is called unconditionally after tearDownClass
is called, so it can also be called as a replacement and/or safer
tearDownClass.
odoo/odoo#30688 (6ce2d6efb5) added an
indirection in prefork workers: Python-level signal handlers are
delayed until native calls have ended (e.g. accept() or
execute()). Running the actual work in a sub-thread allowed the main
thread to handle signals in all cases.
However there is apparently an issue with SIGXCPU on linux (possibly
other cases as well): SIGXCPU is delivered to the child thread (if
possible?) and Thread.join apparently stops it from redelivered to
the main thread (Thread.join is signal-interruptible since 3.2 but
possibly not Python-interruptible).
Blocking SIGXCPU on the child thread causes the OS to deliver on the
main thread and fixes the issue.
Also split set_limits so it sets the signal handler in the parent
thread but properly updates the soft limit in the child after each
request, as the goal is to put a hard limit on the CPU time per
request, not on the worker. 6ce2d6ef would set the limit once then
never update it, likely cycling workers more than desired.
While at it:
* block other signals with a handler set, they seem to work
regardless on linux but other OS may have a different way of
dispatching process-directed signals
* unset signals which are set by the prefork server but whose
set behavior makes no sense in workers:
- TERM and CHLD were already unset
- HUP is used to restart the server, workers can just be killed
- TTIN and TTOU configure the number of workers
closesodoo/odoo#39731
X-original-commit: 549bd199bad269e4e28efac933efac3f41495877
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
* walksymlinks is useless, os.walk got followlinks in 2.6
* tempdir is redundant with tempfile.TemporaryDirectory added in 3.2
* listdir(recursive=False) is just os.listdir (I guess recursive=True
is somewhat useful)
* zip_dir is *not* redundant with shutil.make_archive:
- zip_dir handles the archived root differently
- zip_dir sorts files, make_archive sorts directories
- make_archive explicitly adds entries for directories (including
empty), sorts directories, doesn't sort files; zip_dir sorts
files (including with a custom key function) but ignores directories
- zip_dir filters out a bunch of trash files
closesodoo/odoo#39573
Signed-off-by: Christophe Simonis <chs@odoo.com>
Werkzeug 0.15 modified ProxyFix such that by default it only forwards
the REMOTE_ADDR when enabled, whereas before 0.15 it would also
forward scheme and host. This breaks proxied odoo as the base url
becomes incorrect (cf #34412).
Use properly configured ProxyFix when running with werkzeug 0.15, old
configuration otherwise.
Backport of 4057227def which was merged
in master, because many people apparently run 0.15 now.
Closes#35085closesodoo/odoo#36212closesodoo/odoo#37708
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
So the logs contain some indication as to what was exceeding the limits.
closesodoo/odoo#37303
X-original-commit: 0a266f444a0abda026d09ad88024dca1c50c92b9
Signed-off-by: Denis Vermylen <Icallhimtest@users.noreply.github.com>
omission in d26e253edd
kill -3 (SIGQUIT) is not processed like the other signals so it doesn't
go through this bit of code, but leaving the AttributeError lurking
there is not a good idea.
closesodoo/odoo#37221
X-original-commit: ac65ef0208a5ec814d263aeb44a98b47a5ee3947
Signed-off-by: Denis Vermylen <Icallhimtest@users.noreply.github.com>
Initial bug report, full of red herrings:
receive two support requests by email while on Odoo.sh, the first creates a
ticket, while the second replies to the server with the error:
550 Requested action not taken: mailbox unavailable
When a request is done, the service check function tries each request multiple
times when getting Operational errors.
When two email are received within a 'short' amount of time (e.g .1 second)
the two requests are initiated with a new cursor.
When both mails generate the creation of a new record of the same model,
the second cursor is going to fail because of an:
ERROR: could not serialize access due to concurrent update
However, the record we used in the computations needed in the creation of the
second record have been added to the environment.
When that second creation is retried, it will operate on the same records,
and find them in the todo, with the environment that was obtained for the first
trial. Since the cursor is closed, this crashes.
To explain the bug report, Odoo.sh transfers the mail through an xmlrpc
to process the mail.
Because of this crash, the server interprets the lack of well-formed response
by a generic 'mailbox unavailable'.
opw 2060476
closesodoo/odoo#37165
X-original-commit: 845eecdfa7b27d84862e94f4b63d40a851e0c410
Signed-off-by: Nans Lefebvre (len) <len@odoo.com>
[PEP-594] is deprecating the `imp` module, that module is used in
`module.py` in order to dynamically import addons using any of the
`odoo.addons` or `openerp.addons` import anchor.
We are deprecating `openerp` module/addons imports in v13 in order to
remove the support in v14 and greatly simplify how modules/addons are
loaded. If you are still using the old `import openerp` or `import
openerp.addons`, `import odoo` and `import odoo.addons` are drop-in
replacements.
The `odoo.modules.module.ad_paths` addon paths list has been deprecated
too. The list is now accessible on `odoo.addons.__path__` where they
are now directly loaded [2].
See also:
[PEP-594]: https://python.org/dev/peps/pep-0594/
[2]: https://packaging.python.org/guides/packaging-namespace-packages/closesodoo/odoo#36597
Task: 2003936
Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
Purpose
=======
When trying the translate a sql_contraint message after an integrity error
we use the current cursor in a 'with', just to make a SELECT query.
That way, the cursor is commited at the end of the with statement, trying
to commit the inconsitent state of the cursor that was the reason of the
integrity error.
Just close the cursor, as we only fetch translations from the database.
closesodoo/odoo#36458
Signed-off-by: Yannick Tivisse (yti) <yti@odoo.com>
[PEP-594] is deprecating the `imp` module, that module is used in
`module.py` in order to dynamically import addons using any of the
`odoo.addons` or `openerp.addons` import anchor.
We are deprecating `openerp` module/addons imports in v13 in order to
remove the support in v14 and greatly simplify how modules/addons are
loaded. If you are still using the old `import openerp` or `import
openerp.addons`, `import odoo` and `import odoo.addons` are drop-in
replacements.
The `odoo.modules.module.ad_paths` addon paths list has been deprecated
too. The list is now accessible on `odoo.addons.__path__` where they
are now directly loaded [2].
See also:
[PEP-594]: https://python.org/dev/peps/pep-0594/
[2]: https://packaging.python.org/guides/packaging-namespace-packages/closesodoo/odoo#36597
Task: 2003936
Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
Werkzeug 0.15 modified ProxyFix such that by default it only forwards
the REMOTE_ADDR when enabled, whereas before 0.15 it would also
forward scheme and host. This breaks proxied odoo as the base url
becomes incorrect (cf #34412).
Use properly configured ProxyFix when running with werkzeug 0.15, old
configuration otherwise.
Closes#35085closesodoo/odoo#36212
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
* fix the gevent worker "logging" everything to stderr and ignoring
the logging configuration: passing loggers to WSGIServer means it
uses those loggers instead of writing directly to stderr
* fix the gevent worker logging the wrong client address when behind a
reverse proxy: it would log the client_address, immitate the
behaviour of werkzeug which goes and gets whatever REMOTE_ADDR is
set on the environment, that way it gets whatever update were
performed by ProxyFix (if any)
Fixes#32914closesodoo/odoo#36233
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
When an `IntegrityError` is raised, the user receives a cryptic error
message which doesn't provide much valuable information in order to
solve the problem.
However, such an error is raised in 3 cases:
1. A mandatory field is not provided at creation/update.
2. The deletion of a record makes a mandatory Many2one field NULL on a
referenced table (`ON DELETE SET NULL` on a not nullable field).
3. The deletion of a record raises a `ON DELETE RESTRICT` foreign-key
constaints.
In the first and second cases, we provide the table and field on which
the `NOT NULL` constaint is raised. We also suggest to archive the
record in case of a deletion.
In the third case, we provide the table and the constraint raised. We
also suggest to archive the record.
Notes:
- The IDs of the records causing the issue is not provided since the
information is not provided by PostgreSQL.
- Although cases 2 and 3 have a different root cause, the error
message raised is very similar. Indeed, from an end-user perspective
the solution is identical: archive the record. Another solution would
also be to manually edit the records raising the error, but most of
the time this is not an option: these records are locked and cannot be
edited anymore.
task-1970853
Closes#32949closesodoo/odoo#33922
Signed-off-by: Nicolas Martinelli (nim) <nim@odoo.com>
Co-authored-by: Sapan Zaveri <sza@odoo.com>
Co-authored-by: Pragya Ladda <pla@odoo.com>
1. Make test logs clearer & remove redundancies
Instead of having an ERROR log right when the test fails then print
the useful / relevant information at the end of the test suite,
immediately print the traceback. Keep the final summary. Also avoids
having to wait for the entire test suite to end before a dev' can know
the failure details of a specific test.
Done by working at a lower level and replacing the custom
test stream mess by a custom Result class which prints and formats the
information we want. Replace TextTestRunner by a bare-bones custom
Runner object to tie it in.
2. Provide useful location information on test failure
Leverage the work above to log the test function's failure location:
previously logging would point to within TestStream which is not
useful.
Here, on failure the traceback is used to discover the caller info and
point to the test line which fails instead. similar to unittest's
_exc_info_to_string (https://github.com/python/cpython/blob/93e8aa62cfd0a61efed4a61a2ffc2283ae986ef2/Lib/unittest/result.py#L173).
3. Replace direct logging in browser_js by raising errors
Properly marks the test as in error, and the error traceback points to
the tour definition / launcher (python side) rather than common.py
and/or module.py.
Also removes unused dbname parameter that was added in
/278ed718e9805edf088642ba10d3b7c4e5716c31/openerp/modules/module.py#L361
for nor visible reason
closesodoo/odoo#34996
Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
We add two warnings to make the --test-file argument more user friendly:
- one when the test file does not exist
example:
./odoo-bin -d master --test-file ./addonss/stock/tests/test_quant.py
before:
INFO master odoo.service.server: loading test file ./addonss/stock/tests/test_quant.py
after:
WARNING master odoo.service.server: test file ./addonss/stock/tests/test_quant.py cannot be found
- one when the test file is not a python file
In both cases, nothing was being tested but the server did not say why.
closesodoo/odoo#34808
Signed-off-by: Xavier Dollé (xdo) <xdo@odoo.com>
Store the SQL constraint message directly in the field `message` of the
corresponding `ir.model.constraint` record, and manage translations from
there. Those records are given an XML id in order to be tracked in PO
files. The translation type 'sql_constraint` is removed.
If a signal is received during the worker _runloop sleep,
the worker will be marked as alive=False but process_work
will still be called once.
This commit prevents that by checking the worker state before
calling process_work.
closesodoo/odoo#31885
Signed-off-by: Olivier Dony (odo) <odo@openerp.com>
Avoid potential tracebacks from the FSWatcher's thread being killed.
Drawback: Server shut down can have an extra small delay
(only applies when the --dev=reload option is given)
closesodoo/odoo#31855
Signed-off-by: Christophe Simonis <chs@odoo.com>
Add the alternative of inotify instead of watchdog to watch the addons
paths the server was started with.
Reason: watchdog spawns 2 threads per path to watch. When there are a
lot of addons paths, this can become too costly. With inotify we watch
all the repositories in a single thread.
https://github.com/dsoprea/PyInotify
installation:
pip install inotify
Adrian Torres