Uploaded image for project: 'Thrift'
  1. Thrift
  2. THRIFT-3224

Fix TNamedPipeServer unpredictable behavior on accept

VotersStop watchingWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Critical
    • Resolution: Fixed
    • 0.9.2
    • 0.9.3
    • C++ - Library
    • Windows

    • Patch Available
    • Patch, Important

    Description

      Application bahavior utilizing TNamedPipeServer is unpredictable due misuse of TAutoHandle.

      Project uses TAutoHandle class, an analogy of std::unique_ptr, for managing WIN32 handles. The dangerous members of this concept are: the direct getter "HANDLE TAutoHandle::h" and release method "void __thiscall TAutoHandle::release()"

      Below code citation introduces serous bug:

      {
          TAutoCrit lock(pipe_protect_);
          GlobalOutput.printf("Client connected.");
          shared_ptr<TPipe> client(new TPipe(Pipe_.h));
          Pipe_.release();
      }
      

      The getter is used in TNamedPipeServer::acceptImpl() to pass internal handle value to c-tor of TPipe and just after c-tion HANDLE__thiscall TAutoHandle::release() is called to release ownership. That means the TPipe object is expected to take ownership of the resource, but if TPipe c-tor throws the d-tor of TAutoHandle is called releasing the resource and the incomplete TPipe object does the same. Since now it is impossible to ensure that the second free of the handle value was not performed on a resource that was opened in meantime by other thread.

      I propose to solve the issue by ensuring the handle is not owned by two objects at a time.

      Attachments

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            pjanicki Paweł Janicki
            pjanicki Paweł Janicki
            Votes:
            0 Vote for this issue
            Watchers:
            5 Stop watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Time Tracking

                Estimated:
                Original Estimate - 2h
                2h
                Remaining:
                Remaining Estimate - 2h
                2h
                Logged:
                Time Spent - Not Specified
                Not Specified

                Slack

                  Issue deployment