Hi,
I run the sample here in debug, and with the release version, but the
problem did not appeared. I understand you are rebuilding the tree during a
double click. This is what you are doing in your program too or just a
test? I wouldn't recommend that.
Anyway, I think we may test for a valid item there, but I don't think
that every call to GETITEM should be tested and simply return doing
nothing.
Is there a way that I can force the sample in a situation that the
problem would be more likely to occur?
Which IUP version are you using?
Best,
Scuri
2018-02-10 22:58 GMT-02:00 Ranier VF <[email protected]>:
> Hi,
> Clearly documents says TVM_GETITEM message can fail.
> https://msdn.microsoft.com/en-us/library/windows/desktop/
> bb773596(v=vs.85).aspx
>
> Iupwin_tree.c there are lot calls to TVM_GETITEM message, without check
> result.
>
> Check if this works?
>
> --- a\src\win\iupwin_tree.c Sat Feb 10 21:58:52 2018
> +++ b\src\win\iupwin_tree.c Sat Feb 10 22:56:21 2018
> @@ -199,14 +199,16 @@
>
> if (hPrevItem)
> {
> - int kindPrev;
> + int kindPrev = ITREE_BRANCH;
> TVITEM tviPrevItem;
>
> /* get the KIND attribute of reference node */
> tviPrevItem.hItem = hPrevItem;
> tviPrevItem.mask = TVIF_PARAM|TVIF_CHILDREN;
> SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&
> tviPrevItem);
> - kindPrev = ((winTreeItemData*)tviPrevItem.lParam)->kind;
> + if (tviPrevItem.lParam) {
> + kindPrev = ((winTreeItemData*)tviPrevItem.lParam)->kind;
> + }
>
> /* Define the parent and the position to the new node inside
> the list, using the KIND attribute of reference node */
> @@ -264,29 +266,41 @@
> static int winTreeIsItemExpanded(Ihandle* ih, HTREEITEM hItem)
> {
> TVITEM item;
> +
> item.hItem = hItem;
> item.mask = TVIF_HANDLE | TVIF_STATE;
> SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item);
> - return (item.state & TVIS_EXPANDED) != 0;
> + if (item.lParam)
> + {
> + return (item.state & TVIS_EXPANDED) != 0;
> + }
> + else
> + {
> + return 0;
> + }
> }
>
> static int winTreeIsBranch(Ihandle* ih, HTREEITEM hItem)
> {
> TVITEM item;
> - winTreeItemData* itemData;
> + int kind = ITREE_BRANCH;
>
> item.mask = TVIF_HANDLE | TVIF_PARAM;
> item.hItem = hItem;
> SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item);
> - itemData = (winTreeItemData*)item.lParam;
> + if (item.lParam)
> + {
> + winTreeItemData* itemData = (winTreeItemData*)item.lParam;
>
> - return (itemData->kind == ITREE_BRANCH);
> + kind = itemData->kind;
> + }
> +
> + return (kind == ITREE_BRANCH);
> }
>
> static void winTreeExpandItem(Ihandle* ih, HTREEITEM hItem, int expand)
> {
> TVITEM item;
> - winTreeItemData* itemData;
>
> iupAttribSet(ih, "_IUPTREE_IGNORE_BRANCH_CB", "1");
> /* it only works if the branch has children */
> @@ -297,15 +311,21 @@
> item.hItem = hItem;
> item.mask = TVIF_HANDLE|TVIF_PARAM;
> SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item);
> - itemData = (winTreeItemData*)item.lParam;
> -
> - if (expand)
> - item.iSelectedImage = item.iImage = (itemData->image_expanded!=-1)?
> itemData->image_expanded: (int)ih->data->def_image_expanded;
> - else
> - item.iSelectedImage = item.iImage = (itemData->image!=-1)?
> itemData->image: (int)ih->data->def_image_collapsed;
> + if (item.lParam)
> + {
> + winTreeItemData* itemData;
>
> - item.hItem = hItem;
> - item.mask = TVIF_HANDLE | TVIF_IMAGE | TVIF_SELECTEDIMAGE;
> + itemData = (winTreeItemData*)item.lParam;
> + if (expand)
> + item.iSelectedImage = item.iImage = (itemData->image_expanded!=-1)?
> itemData->image_expanded: (int)ih->data->def_image_expanded;
> + else
> + item.iSelectedImage = item.iImage = (itemData->image!=-1)?
> itemData->image: (int)ih->data->def_image_collapsed;
> + item.mask = TVIF_HANDLE | TVIF_IMAGE | TVIF_SELECTEDIMAGE;
> + }
> + else
> + {
> + item.mask = TVIF_HANDLE;;
> + }
> SendMessage(ih->handle, TVM_SETITEM, 0, (LPARAM)(LPTVITEM)&item);
> }
>
> @@ -493,13 +513,17 @@
> HTREEITEM new_hItem;
> TVITEM item;
> TVINSERTSTRUCT tvins;
> - TCHAR title[255];
> + TCHAR title[256];
>
> item.hItem = hItem;
> item.mask = TVIF_HANDLE | TVIF_PARAM | TVIF_IMAGE | TVIF_SELECTEDIMAGE
> | TVIF_TEXT | TVIF_STATE | TVIF_PARAM;
> item.pszText = title;
> - item.cchTextMax = 255;
> + item.cchTextMax = sizeof(title) - 1;
> SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item);
> + if (!item.lParam)
> + {
> + return NULL;
> + }
>
> if (is_copy) /* during a copy the itemdata reference is not reused */
> {
> @@ -551,19 +575,23 @@
> TVITEM item;
> winTreeItemData* itemDataDst;
> int id_new, count, id_src, id_dst;
> + int old_count;
>
> - int old_count = ih->data->node_count;
> + /* Get DST node attributes */
> + item.hItem = hItemDst;
> + item.mask = TVIF_HANDLE | TVIF_PARAM | TVIF_STATE;
> + SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item);
> + if (!item.lParam)
> + {
> + return NULL;
> + }
>
> + old_count = ih->data->node_count;
> id_src = iupTreeFindNodeId(ih, hItemSrc);
> id_dst = iupTreeFindNodeId(ih, hItemDst);
> id_new = id_dst+1; /* contains the position for a copy operation */
>
> - /* Get DST node attributes */
> - item.hItem = hItemDst;
> - item.mask = TVIF_HANDLE | TVIF_PARAM | TVIF_STATE;
> - SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item);
> itemDataDst = (winTreeItemData*)item.lParam;
> -
> if (itemDataDst->kind == ITREE_BRANCH && (item.state & TVIS_EXPANDED))
> {
> /* copy as first child of expanded branch */
> @@ -574,8 +602,7 @@
> {
> if (itemDataDst->kind == ITREE_BRANCH)
> {
> - int child_count = iupdrvTreeTotalChildCount(ih, hItemDst);
> - id_new += child_count;
> + id_new += iupdrvTreeTotalChildCount(ih, hItemDst);
> }
>
> /* copy as next brother of item or collapsed branch */
> @@ -633,6 +660,10 @@
> item.hItem = hItem;
> item.mask = TVIF_HANDLE | TVIF_PARAM | TVIF_IMAGE |
> TVIF_SELECTEDIMAGE | TVIF_STATE;
> SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item);
> + if (!item.lParam)
> + {
> + continue;
> + }
> itemData = (winTreeItemData*)item.lParam;
>
> if (itemData->kind == ITREE_BRANCH)
>
> Best,
> Ranier Vilela
> ________________________________________
> De: 云风 Cloud Wu <[email protected]>
> Enviado: sexta-feira, 9 de fevereiro de 2018 14:08
> Para: [email protected]
> Assunto: [Iup-users] winiup tree bug: Message from an item already deleted
>
> I found a bug can crash the program today. It crash in
>
> static int winTreeCallBranchLeafCb(Ihandle* ih, HTREEITEM hItem)
> @iupwin_tree.c
>
> SendMessage(ih->handle, TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item);
> itemData = (winTreeItemData*)item.lParam;
> if (itemData->kind == ITREE_BRANCH)
>
> SendMessage may return false, and itemData will be NULL .
>
> It may caused by delete nodes in `executeleaf_cb` .
>
> It seems that windows may push a notify message before an item deleted,
> and then it recv this message, but the item is gone.
>
> I try to make a minimal working example , but it doesn't work every time.
>
> -------
> local tree = iup.tree {
> HIDEBUTTONS = "yes",
> HIDELINES = "yes",
> }
>
> local dlg = iup.dialog {
> tree,
> margin = "4x4",
> size = "HALFxHALF",
> shrink="yes",
> title = "Shader Compiler",
> }
>
> local function do_some_systemcall()
> for i=1,100 do
> -- c:\\windows\\win.ini should be exist.
> local f = io.open("c:\\windows\\win.ini","rb")
> if f then
> f:close()
> end
> end
> end
>
> local function rebuild_tree(flag, id)
> tree.delnode0 = "CHILDREN"
> tree.title0 = "foobar"
> if flag then
> tree.addbranch0 = "xxx"
> tree.value = 1
> do_some_systemcall()
> for i=1,3 do
> tree.addleaf1 = "xxx"
> tree.image2 = "IMGCOLLAPSED"
> end
> do_some_systemcall()
> else
> tree.addleaf0 = "Click Me (may crash)"
> tree.image1 = "IMGCOLLAPSED"
> for i=0,3 do
> do_some_systemcall()
> tree.addleaf0 = "xxx"
> tree.image1 = "IMGCOLLAPSED"
> end
> tree.value = id
> end
>
> function tree:executeleaf_cb(id)
> rebuild_tree(not flag, id)
> end
>
> function tree:branchclose_cb(id)
> rebuild_tree(not flag, id)
> return iup.IGNORE
> end
> end
>
> dlg:showxy(iup.CENTER,iup.CENTER)
>
> rebuild_tree(false)
>
> iup.MainLoop()
> iup.Close()
> -------
>
> Run this program, and click the last item (named "Click Me") , it may
> crash on windows 10 64bit （1709/16299.192) .
>
> I think we should check the result of `SendMessage(ih->handle,
> TVM_GETITEM, 0, (LPARAM)(LPTVITEM)&item)` to fix it, or it there a better
> way ?
>
>
> ------------------------------------------------------------
> ------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, Slashdot.org! http://sdm.link/slashdot
> _______________________________________________
> Iup-users mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/iup-users
>
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Iup-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/iup-users
