swaywm / sway

i3-compatible Wayland compositor
https://swaywm.org
MIT License
14.55k stars 1.11k forks source link

Crash resistance against running out of (virtual) memory #3351

Open mstoeckl opened 5 years ago

mstoeckl commented 5 years ago

First, limit the total amount of virtual memory (e.g. with ulimit -v 150000), and then run sway. After opening too many windows (in my example, termite), sway crashes.

sway/wlroots versions: current (1.0-beta.2-70-g4d88c957 / e2c216a4b8480) Stack trace: coredumplog.txt. (Core dump file was not available.) Verbose log: swaylog.txt Configuration file: default.

By default, on 64-bit Linux programs effectively never run out of virtual memory. The main exceptions are when overcommit is disabled or resource/cgroup limits are set. As malloc fails primarily when there is insufficient virtual memory, the allocation error handling code is untested, and hence remains the most likely culprit in this scenario.

Unfortunately, resolving this sort of issue requires significant code rework and testing for marginal stability improvement. By inspection, sway currently has several latent failed-allocation error handling problems, especially with code that uses list_t (see #3257), in the use of heap-allocated struct cmd_results as a return value for checkarg, and (based on the bugs in my own code) probably several in string handling, any of which might induce heap corruption.

Edit: Testing with Failmalloc reveals that return values for the tree-building functions in the JSON serialization code are almost never checked. [Fortunately, one can almost statement by statement translate the functions in ipc-json.c into a two-pass serializer, in which the first pass estimates the required string size, and the second stream writes the contents.]

Edit the second: The following branch tracks in-progress work on rewriting the JSON ipc functions: https://github.com/mstoeckl/sway/commits/proper-serialization , currently at ~30%. (The remainder of the work is straightforward, but as almost every line in ipc-json.c will need to change, config file test cases would be welcome.)

cedws commented 5 years ago

Perhaps you could send a PR to emphasise the need for handling allocation failures in CONTRIBUTING.md?

elfring commented 4 years ago

:crystal_ball: I hope that additional software development approaches can be reconsidered.