You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by "David Riley Coderon (JIRA)" <ji...@apache.org> on 2013/08/16 23:51:48 UTC

[jira] [Created] (THRIFT-2131) Thrift's D bindings: tutorial code has multiple correctness issues

David Riley Coderon created THRIFT-2131:
-------------------------------------------

             Summary: Thrift's D bindings: tutorial code has multiple correctness issues
                 Key: THRIFT-2131
                 URL: https://issues.apache.org/jira/browse/THRIFT-2131
             Project: Thrift
          Issue Type: Bug
          Components: D - Library
    Affects Versions: 0.9
            Reporter: David Riley Coderon


Both the client and the async_client in thrift/tutorial/d have correctness issues, readily apparent from the output of running them against the tutorial server. Seems to occur even back in dmd 2.061.

{code}
you@myhost:~/pkg/thrift/github-latest-thrift/thrift/tutorial/d$ ./async_client
ping()
1 + 1 = 2
Whoa we can divide by 0                //// <<<<<<<<<<<<<<< problem?
15 - 10 = 5
Check log: 5
you@myhost:~/pkg/thrift/github-latest-thrift/thrift/tutorial/d$ ./client
ping()
1 + 1 = 7036616                            ////// <<<<<<<<<<<<<< problem!
Invalid operation: Cannot divide by 0
15 - 10 = 7036616                         ////// <<<<<<<<<<<<<< problem!
Check log: 5
you@myhost:~/pkg/thrift/github-latest-thrift/thrift/tutorial/d$ dmd|head
DMD64 D Compiler v2.061
{code}

notes from David N. :

{code}
On 16 Aug 2013, at 19:33, David N wrote:

    On 16 Aug 2013, at 19:31, David N wrote:

        The async client should definitely throw the exception as well, so there is definitely something wrong.


    I could just reproduce this issue using the second-to-last LDC release on OS X; will have a short look at it.


Oh, derp, this is actually an issue in the async_client code: Thrift structs are passed in D as const(ref), and because async requests are executed … well, asynchronously, overwriting that struct with the (15 - 10) request leads to that being executed twice.

The transition to pass-by-ref was actually done very late in the development process, and apparently nobody really had a closer look at the async_client tutorial output since.

I suppose the proper fix would be to make TAsyncClient accept structs by value, as the actual requests are (potentially) executed in a different thread and sharing data via an un-'shared' reference shouldn't really happen in D.

I wonder what to do about the types that are intrinsically reference types (arrays, …) though. Requiring them to be (tail-)immutable would be one option, albeit a rather harsh one. I'll need to properly think this through later.


David
{code}


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira