2017-07-25 18:10 GMT+09:00 Ozaki Kiichi <gclient.g...@gmail.com>: > > First, the patch seems to prevent Vim from crashing by simply changing > the order of the calls of XCheckTypedEvent(). This happens to work for the > purpose because the call for PropertyNotify happens to catch the INCL > PropertyNotify which is to be sent to the requestor prior to the actual > data transaction when the selection is large relative to max-request-size > (implementation dependent). The event is, however, not handled properly > because, according to ICCCM, the property has to be deleted, yet never by > Vim actually. We have a server resource leak here. > > > > Secondly, the same manual also says that after deleting the property, > the transaction of the body of the selection data begins and then finishes > with the owner's sending a zero-length property to the requestor. It's > requestor's responsibility to delete the zero-length property; however, Vim > will never send it. Another resource leak takes place in the peer client. > > Changing the order of XCheckTypedEvent() is a fix to paste correctly, not > directly related with preventing crash. > > I might be wrong but, as far as I browsed libxt code, INCR processes seem > to be handled by the inside of libxt. > > > Lastly, since Vim didn't know what the zero-length property means, it > tried to free the same memory block twice, resulting in crash. With the > patch, the memory block is statically allocated to avoid the crash. > However, if Vim understood the fact that it would get PropertyNotify twice > per large selection at the end of the transaction, this change wouldn't be > necessary at all. > > Does the above description (about zero-length property, PropertyNotify) > explain the requestor-side behavior? > A crash happens in the owner side Vim, so I think this kind (owner-side) > of fix is need in addition to correcting the way of handling property at > requestor. > > Note: > Crash when receiving selection-request from some other application. > (Therefore I think owner-side fix is need) > > Example: vim (prior than v8.0.0737) and xclip > ---- > $ vim --clean -c 'h eval.txt' > > and input `ggvG$` (on visual mode, selecting all) > > and on another terminal: > > $ while :; do xclip -o -selection primary | wc -c; sleep 1; done > > (when xclip stalls, do Ctrl-C and restart) > ---- >
IMHO, you'd rather need to explain to us first why your patch is OK, which should have been done when you posted it, Because of lack of your explanation, I had to read bulky references, manuals and source code to review it while I was quite busy for repairing and restoring my PC. Anyway, arguing only about my views won't help to validate your patch. There's one thing I didn't write in my previous mail: You included an empty stub as XtSelectionDoneProc in the patch. A manual says, when XtSelectionDoneProc is non-NULL, the selection owner owns the storage containing the converted selection value. We now have an XtSelectionDoneProc which does nothing but claims the ownership of the storage. What is this for? The implication is clear... > > -- > -- > 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 > > --- > You received this message because you are subscribed to the Google Groups > "vim_dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to vim_dev+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > -- -- 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 --- You received this message because you are subscribed to the Google Groups "vim_dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to vim_dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
