Kazunobu Kuriyama wrote: > 2017-07-25 18:10 GMT+09:00 Ozaki Kiichi <[email protected]>: > > > > 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...
My understanding was the combination of making some data static, instead of allocating it, and using the DoneProc, means that the data won't be freed unexpectedly. It would be good to include comments in the code to explain these things. Especially since it's likely other people go in there trying to fix their bug or add a new feature. -- "Marriage is when a man and woman become as one; the trouble starts when they try to decide which one" /// Bram Moolenaar -- [email protected] -- http://www.Moolenaar.net \\\ /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\ \\\ an exciting new programming language -- http://www.Zimbu.org /// \\\ help me help AIDS victims -- http://ICCF-Holland.org /// -- -- 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 [email protected]. For more options, visit https://groups.google.com/d/optout.
