On 17 December 2011 21:49, Bram Moolenaar wrote: > > Björn Winckler wrote: > >> I received a report on Vim crashing on the MacVim issue tracker [1] >> and since I could reproduce the problem I decided to try to track it >> down. >> >> The problem is a NULL point access in plines_win_col(), specifically the call >> >> s = ml_get_buf(wp->w_buffer, lnum, FALSE); >> >> is done when w_buffer is NULL and ml_get_buf() assumes its first >> parameter to be non-NULL. >> >> I did a git-bisect and found that the following commit introduced the >> problem: >> >> updated for version 7.3.306 >> Problem: When closing a window there is a chance that deleting >> a scrollbar >> triggers a GUI resize, which uses the window while it is not in a >> valid state. >> Solution: Set the buffer pointer to NULL to be able to detect the >> invalid >> situation. Fix a few places that used the buffer pointer >> incorrectly. >> >> My take on this is that this commit is very dangerous. Some places in >> the code (like the one above) assumes that wp->w_buffer always is >> non-NULL but after 7.3.306 this is no longer the case. Since I don't >> think I'll be able to spot all places where this assumption is made I >> decided against trying to write a patch and instead ask what to do >> about this: revert 7.3.306 or try to fix all places in the code that >> it breaks? >> >> For reference I've pasted the backtrace from the crash below. > > The reason to put NULL in this pointer is that otherwise an invalid > pointer would be used. Now you can see where the NULL is used and find > out why this happens and solve the problem. Previously these problems > were very hard to debug. > > You will have to put in checks for w_buffer being null, and bailing out > early. Especially in event handlers, e.g. for window resizing. > It should not be needed in all places where w_buffer is used.
OK, I guess that should have been clear to me upon reading the commit message but I somehow missed that. I created a patch for the specific problem that I dug up (patch is attached). The problem was that a window was never closed, so when something iterated over the window list there would be a crash. The attached patch detects this situation and closes the window before any damage can be done. I had to patch win_close() as well to not assume that w_buffer is non-NULL or it would crash when trying to close the window. A better solution may be to always close a window before closing its buffer but that would probably lead to many changes wherever a window/buffer is closed so I'm not sure that is preferable. This problem is 100% reproducible following the instructions in the original issue report, so you may want to debug this yourself to find a better solution. However, at least this fixes one immediate crashing bug. Björn -- You received this message from the "vim_dev" maillist. Do not top-post! Type your reply below the text you are replying to. For more information, visit http://www.vim.org/maillist.php
0001-Don-t-leave-window-open-after-closing-its-buffer.patch
Description: Binary data
