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 <ranier_...@hotmail.com>:

> 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 <clou...@gmail.com>
> Enviado: sexta-feira, 9 de fevereiro de 2018 14:08
> Para: iup-users@lists.sourceforge.net
> 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
> Iup-users@lists.sourceforge.net
> 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
Iup-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/iup-users

Reply via email to