Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#455 BugFix: Random crash when opening recent file list files several times

Next_release
closed
nobody
None
5
2013-09-21
2013-02-21
FLS
No

When a file is opened/closed several times using the recent file list in the files menu, then, NPP crashes occasionally after 3 to 7 cycles.
The bug is in NppIO.cpp doOpen() function because there the function _lastRecentFileList.remove() deletes the buffer (in the recent file list) where *fileName pointer points at.
NPP then crashes randomly at generic_string gs_fileName = fileName; below where *fileName is used.
Therefore, _lastRecentFileList.remove(); statement is moved down, where *fileName is no more used.

See patch NppPatch_6.3_FixCrashOpenRecentFileListFiles.patch

FLS

1 Attachments

Discussion

  • Don HO
    Don HO
    2013-02-23

    Thank you for the patch.

    I cannot reproduce the crash at all.
    Is there any way to reproduce the crash steadily?

    Don

     
    Last edit: Don HO 2013-02-23
  • FLS
    FLS
    2013-02-23

    I don’t think so. I did further tests. The crash (access violation) appeared randomly after several cycles when I compiled SVN-revision 1024 (using VS2008; WinXP). In my case, using my compiled version in debug-mode, the crash occurs mostly between 3 to 7 cycles closing/opening a file using the recent file list. When I am starting the very same notepadPlus_Debug.exe from Windows Explorer (and not with F5 from within VS2008), I could cycle up to 20 times without any crash. The same is true for the compiled release version. I cleaned up the VS2008-project and compiled all files again, but the crash behaviour was still there. I downloaded NPP 6.3 and could not reproduce the crash!

    However, when analysing (debugging) the code in NppIO.cpp doOpen() function and observing the contents of *fileName pointer, you can see that the contents of fileName pointer vanishes after _lastRecentFileList.remove() but fileName is used afterwards. In my opinion, this is an unsafe situation, which could be solved by moving the call to _lastRecentFileList.remove(longFileName); a little bit down.

    Furthermore, I could not get the reason of the following code:

    size_t res = gs_fileName.find_first_of(UNTITLED_STR);

    if (res != string::npos && res == 0)
    {
    fileName2Find = fileName;
    }
    else
    {
    fileName2Find = longFileName;
    }

    Why is there a difference between fileName and longFileName if there is the string “new “ at the beginning of the fileName-string??

    But even that does not work because, if gs_fileName” is for example “01w_new 3_" .find_first_of(new )” will return 2, which is the position of the “w” in “01w” and not the position of “new “!! The function .find_first_of() looks up the first occurrence of any character of character set “new “ within gs_fileName.
    So any filename with an “n”, “e”, “w” or blank as the first character will fulfil the if-condition.

    FLS

     
    • Don HO
      Don HO
      2013-09-21

      The code that you don't see the reason does just the thing following:
      if UNTITLED_STR is found in the beginning then "fileName" is just fine to be used, otherwise longFileName which contains the long file name format of full file path will be used.

      Regarding the shift of line "_lastRecentFileList.remove(longFileName);",
      I don't see the reason why fileName is corrupted and why shifting this line can prevent from fileName content corrupted, neither I can reproduce the crash with my VS2005 on debug.

      Feel free to reopen this patch if there's any way to reproduce the crash afterward.

      Thank you anyway for your contribution.

       
      • FLS
        FLS
        2013-09-22

        Just observe the content change of “fileName” when the function _lastRecentFileList.remove(longFileName); has been executed. I’ve provided two screen shot images during debugging, showing the content before and after.
        Maybe other compilations are showing different effects.

         
  • Don HO
    Don HO
    2013-09-21

    • status: open --> closed
    • Priority: 6 --> 5