[
https://issues.apache.org/jira/browse/THRIFT-3224?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14621612#comment-14621612
]
Ben Craig commented on THRIFT-3224:
-----------------------------------
updated my pull request. Take a look at that when you get a chance. I haven't
had time to test the changes. I'm hoping to add some tests tomorrow, and
hopefully push this through at that point.
> Fix TNamedPipeServer unpredictable behavior on accept
> -----------------------------------------------------
>
> Key: THRIFT-3224
> URL: https://issues.apache.org/jira/browse/THRIFT-3224
> Project: Thrift
> Issue Type: Bug
> Components: C++ - Library
> Affects Versions: 0.9.2
> Environment: Windows
> Reporter: Paweł Janicki
> Assignee: Paweł Janicki
> Priority: Critical
> Labels: patch
> Attachments:
> 0001-THRIFT-3224.-cpp-Fix-TNamedPipeServer-unpredictable-.patch,
> 0002-THRIFT-3224.-cpp-Fix-TNamedPipeServer-unpredictable-.patch,
> 0003-THRIFT-3224.-cpp-Fix-TNamedPipeServer-unpredictable-.patch,
> 0004-THRIFT-3224.-cpp-Fix-TNamedPipeServer-unpredictable-.patch
>
> Original Estimate: 2h
> Remaining Estimate: 2h
>
> 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:
> {code:java}
> {
> TAutoCrit lock(pipe_protect_);
> GlobalOutput.printf("Client connected.");
> shared_ptr<TPipe> client(new TPipe(Pipe_.h));
> Pipe_.release();
> }
> {code}
> 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.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
