[ 
https://issues.apache.org/jira/browse/THRIFT-491?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12708245#action_12708245
 ] 

Rush Manbert commented on THRIFT-491:
-------------------------------------

The changes to Monitor look good and work as expected and there's now a Monitor 
test.

I got looking at the writerThread in TFileTransport. Initially, I thought I'd 
define a new Runnable-derived class and move the thread method into it as its 
run() method. The problem with that is that the thread function uses lots of 
the data members and functions in TFileTransport.

I am now considering whether I can just add Runnable as a base class of 
TFileTransport, rename writerThread() as run(), and just go with the "mimimum 
disturbance" approach. The biggest problem that I see is that Iwould neede to 
add boost::enable_shared_from_this as a base class and use it to create the 
Thread, without being able to guarantee that the TFileTransport object is owned 
by a boost::Shared_ptr.

I could solve that by taking the constructor private and providing a static 
method that returns bost::shared_ptr<TFileTransport>, but that breaks existing 
code.

The nastier route is to do something like my original plan. Make a separate 
Runnable class for the writerThread, and construct it with a pointer to the 
TFileTransport that owns it, then make the thread class be a friend of 
TFileTransport and just reach in as if run() were a member of TFileTransport. 
If I wanted to make it cleaner I could put all the shared data in a separate 
structure that is shared between the writerThread class and the TFileTransport, 
and make sure that TFileTransport has an API that supports all the calls that 
writerThread needs to make. It's more work, and more disturbance to the 
original design, but maybe safer. Or maybe not.

Does anyone with knowledge of TFileTransport care to comment? Please?

> Ripping raw pthreads out of TFileTransport and associated test issues
> ---------------------------------------------------------------------
>
>                 Key: THRIFT-491
>                 URL: https://issues.apache.org/jira/browse/THRIFT-491
>             Project: Thrift
>          Issue Type: Question
>          Components: Library (C++)
>    Affects Versions: 0.1
>         Environment: Mac OS X 10.5.6
>            Reporter: Rush Manbert
>
> The last piece of replacing the pthread-based threading implementation with a 
> Boost threads implementation is to replace all of the raw ptherad mutex and 
> condition usage in TFileTransport.
> I think the best way to proceed is to define a Condition class, similar to 
> the way there is a Mutex class defined, give it a generic API that satisfies 
> the demands of TFileTransport, then implement it both ways. You would need to 
> construct a Condition object with a Mutex object reference, or probably a 
> weak_ptr<Mutex> so we can know that the condition waits are safe. I'll add a 
> comment with the API I work out.
> However, my main concern is how to test the new code. It looks like I can use 
> stress-test, but iI was hoping that someone who is familiar with the app 
> could give me some pointers or a test procedure that exercises the threading 
> code.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to