[jira] [Commented] (THRIFT-1768) unions can't have required fields (Compiler)
[ https://issues.apache.org/jira/browse/THRIFT-1768?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13509770#comment-13509770 ] Kenjiro Fukumitsu commented on THRIFT-1768: --- I think at the entrance of union's property setter, we should call a function to clear all the values and issets. unions can't have required fields (Compiler) Key: THRIFT-1768 URL: https://issues.apache.org/jira/browse/THRIFT-1768 Project: Thrift Issue Type: Bug Components: Compiler (General) Affects Versions: 0.8, 0.9 Reporter: Jens Geyer Assignee: Jens Geyer Fix For: 1.0 Attachments: THRIFT-1768_unions_cant_have_required_fields_Compiler.patch Required fields within unions should issue a warning. First, required makes not much sense with unions. Next, most languages are already treating them as optional anyway, effectively ignoring the required attribute. -- 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
[jira] [Updated] (THRIFT-1753) Multiple C++ Windows, OSX, and iOS portability issues
[ https://issues.apache.org/jira/browse/THRIFT-1753?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ben Craig updated THRIFT-1753: -- Attachment: cleaner_port4.patch Same as last patch, but now starting the TFileTransport flush thread, instead of just creating it and letting it do nothing. Multiple C++ Windows, OSX, and iOS portability issues - Key: THRIFT-1753 URL: https://issues.apache.org/jira/browse/THRIFT-1753 Project: Thrift Issue Type: Bug Components: C++ - Library Affects Versions: 0.9, 1.0 Environment: Windows MSVC10, MSVC11 OSX GCC-4.2 iOS Clang-4.0 Reporter: Ben Craig Fix For: 1.0 Attachments: cleaner_port3.patch, cleaner_port4.patch These are all in the C++ library. Here is a summary of what I changed. All of these fixes make a ~5000 line patch (though a lot of that is deleted lines). * General cleanup of the msvc project * Using HAVE_CONFIG_H instead of force including files * Getting rid of some unnecessary files (stdafx.*, TargetVersion.h) * Significant rework of windows portability. No longer using config.h and force_inc.h to make Windows look like *nix. Instead, making lots of Thrift specific #defines that are vaguely *nixy, and having those forward to *nix or Windows stuff appropriately. For example, THRIFT_CTIME_R calls ctime_r on *nix, and a wrapper thrift_ctime_r on Windows. The old approach doesn't work when multiple libraries attempt the same trick. For example, if openssl #defined errno to ::WSAGetLastError() as well, then that would cause problems. * Adding preprocessor flag that can optionally squelch console output. Default behavior is unchanged. Console output is great for home deployed server apps, but it looks unprofessional for consumer apps. * Adding THRIFT_UNUSED_VARIABLE helper macro, to aid in squelching warnings. * Adding redirector header for functional and tr1/functional. Since namespaces aren't consistent (std vs std::tr1), I have added symbols to the apache::thrift::stdcxx namespace. This is important for Clang / iOS support * usleep and sleep on Windows were both sleeping in milliseconds. sleep now correctly sleeps for seconds, and usleep attempts to sleep for microseconds (after converting very coarsely to milliseconds). * Adding support for using C++11 std::thread (and mutex, and monitor). Thrift already supported boost::thread and posix threads. Clients that use std::thread no longer need built boost libraries. The boost headers are sufficient for them. Switching from boost::thread to std::thread resulted in a ~50k reduction in exe size in my tests. By default, msvc10 and below will use boost::thread, msvc11 and up will use std::thread. * Fixing more 64-bit socket truncation issues in non-blocking server and ssl support. openssl itself has socket truncation issues, so I could not fix them all. * Fixed THRIFT-1692 SO_REUSEADDR allows for socket hijacking on Windows . Now using SO_EXCLUSIVEADDRUSE on windows, and SO_REUSEADDR on *nix. * Making TFileTransport use thrift style threads instead of redoing all the pthread+boost stuff itself. * Includes, and builds upon THRIFT-1740 (Make C++ library build on OS X and iOS) * Moved several functions out of thrift/windows/config.h, and into other thrift/windows headers. * Using built-in stdint.h on Windows if available (by checking HAVE_STDINT_H) and using boost typedefs otherwise. -- 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
[jira] [Updated] (THRIFT-1750) Make compiler build cleanly under visual studio 10
[ https://issues.apache.org/jira/browse/THRIFT-1750?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ben Craig updated THRIFT-1750: -- Attachment: (was: compiler_cleanup.patch) Make compiler build cleanly under visual studio 10 -- Key: THRIFT-1750 URL: https://issues.apache.org/jira/browse/THRIFT-1750 Project: Thrift Issue Type: Bug Components: Compiler (General) Affects Versions: 0.9, 1.0 Environment: Windows, MSVC10 Reporter: Ben Craig Fix For: 1.0 Attachments: compiler_cleanup.patch Building the thrift compiler in Visual Studio 10 causes a lot of warnings. -- 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
[jira] [Updated] (THRIFT-1750) Make compiler build cleanly under visual studio 10
[ https://issues.apache.org/jira/browse/THRIFT-1750?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Ben Craig updated THRIFT-1750: -- Attachment: compiler_cleanup.patch Putting a newline at the end of platform.h that was accidentally omitted. Make compiler build cleanly under visual studio 10 -- Key: THRIFT-1750 URL: https://issues.apache.org/jira/browse/THRIFT-1750 Project: Thrift Issue Type: Bug Components: Compiler (General) Affects Versions: 0.9, 1.0 Environment: Windows, MSVC10 Reporter: Ben Craig Fix For: 1.0 Attachments: compiler_cleanup.patch Building the thrift compiler in Visual Studio 10 causes a lot of warnings. -- 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
[jira] [Commented] (THRIFT-1767) unions can't have required fields (Delphi)
[ https://issues.apache.org/jira/browse/THRIFT-1767?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13509799#comment-13509799 ] Kenjiro Fukumitsu commented on THRIFT-1767: --- I think it is nicer if union's property setter clear all other values. Here is experimental patch for it. unions can't have required fields (Delphi) -- Key: THRIFT-1767 URL: https://issues.apache.org/jira/browse/THRIFT-1767 Project: Thrift Issue Type: Bug Components: Delphi - Compiler Affects Versions: 0.8, 0.9 Reporter: Jens Geyer Assignee: Jens Geyer Fix For: 1.0 Attachments: THRIFT-1767_unions_cant_have_required_fields.patch Required fields within unions should be treaten as optional, the union construct would not make much sense otherwise. -- 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
[jira] [Updated] (THRIFT-1767) unions can't have required fields (Delphi)
[ https://issues.apache.org/jira/browse/THRIFT-1767?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Kenjiro Fukumitsu updated THRIFT-1767: -- Attachment: THRIFT-1767_union_clear_value_experimental.patch unions can't have required fields (Delphi) -- Key: THRIFT-1767 URL: https://issues.apache.org/jira/browse/THRIFT-1767 Project: Thrift Issue Type: Bug Components: Delphi - Compiler Affects Versions: 0.8, 0.9 Reporter: Jens Geyer Assignee: Jens Geyer Fix For: 1.0 Attachments: THRIFT-1767_union_clear_value_experimental.patch, THRIFT-1767_unions_cant_have_required_fields.patch Required fields within unions should be treaten as optional, the union construct would not make much sense otherwise. -- 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
[jira] [Comment Edited] (THRIFT-1767) unions can't have required fields (Delphi)
[ https://issues.apache.org/jira/browse/THRIFT-1767?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13509799#comment-13509799 ] Kenjiro Fukumitsu edited comment on THRIFT-1767 at 12/4/12 3:30 PM: I think it is nicer if union's property setter clear all other values. Here is experimental patch for it(including Jens's change). was (Author: kenjiro): I think it is nicer if union's property setter clear all other values. Here is experimental patch for it. unions can't have required fields (Delphi) -- Key: THRIFT-1767 URL: https://issues.apache.org/jira/browse/THRIFT-1767 Project: Thrift Issue Type: Bug Components: Delphi - Compiler Affects Versions: 0.8, 0.9 Reporter: Jens Geyer Assignee: Jens Geyer Fix For: 1.0 Attachments: THRIFT-1767_union_clear_value_experimental.patch, THRIFT-1767_unions_cant_have_required_fields.patch Required fields within unions should be treaten as optional, the union construct would not make much sense otherwise. -- 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
[jira] [Commented] (THRIFT-1481) Unix domain sockets in C++ do not support the abstract namespace
[ https://issues.apache.org/jira/browse/THRIFT-1481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13509818#comment-13509818 ] Ben Craig commented on THRIFT-1481: --- I should be able to provide a .patch for this once THRIFT-1753 is resolved. I haven't quite figured out a good workflow for managing incremental patches, and with the impending switch to Git, I'm hesitant to figure out an SVN based approach now. Unix domain sockets in C++ do not support the abstract namespace - Key: THRIFT-1481 URL: https://issues.apache.org/jira/browse/THRIFT-1481 Project: Thrift Issue Type: Bug Components: C++ - Library Affects Versions: 0.8 Reporter: Asad Saeed Priority: Minor Linux provides the ability to create a Unix Domain socket in an abstract namespace independent of the filesystem. A abstract namespace is specified by having the sockaddr_un.sun_path start with a NULL character. TServerSocket and TSocket both utilize snprintf when writing to the sockaddr_un structure, which stops at the first NULL character. Abstract namespace support can be added by using memcopy instead. -- 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
[jira] [Commented] (THRIFT-1481) Unix domain sockets in C++ do not support the abstract namespace
[ https://issues.apache.org/jira/browse/THRIFT-1481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13509947#comment-13509947 ] Vlad Troyanker commented on THRIFT-1481: +1 This is important to interoperate with Java servers. Java local sockets (server-side) support only Abstract namespaces. Unix domain sockets in C++ do not support the abstract namespace - Key: THRIFT-1481 URL: https://issues.apache.org/jira/browse/THRIFT-1481 Project: Thrift Issue Type: Bug Components: C++ - Library Affects Versions: 0.8 Reporter: Asad Saeed Priority: Minor Linux provides the ability to create a Unix Domain socket in an abstract namespace independent of the filesystem. A abstract namespace is specified by having the sockaddr_un.sun_path start with a NULL character. TServerSocket and TSocket both utilize snprintf when writing to the sockaddr_un structure, which stops at the first NULL character. Abstract namespace support can be added by using memcopy instead. -- 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
[jira] [Commented] (THRIFT-1767) unions can't have required fields (Delphi)
[ https://issues.apache.org/jira/browse/THRIFT-1767?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13509975#comment-13509975 ] Jens Geyer commented on THRIFT-1767: I'm not so sure. {code} Union { 1 : i32 one, 2 : i32 two = 3 } {code} Would be ok. Only {code} Union { 1 : i32 one = 4, 2 : i32 two = 3 } {code} would produce something unexpected. Depending on the impl either more than one field would be set, or only the last value. But yes, a warning should be emitted. unions can't have required fields (Delphi) -- Key: THRIFT-1767 URL: https://issues.apache.org/jira/browse/THRIFT-1767 Project: Thrift Issue Type: Bug Components: Delphi - Compiler Affects Versions: 0.8, 0.9 Reporter: Jens Geyer Assignee: Jens Geyer Fix For: 1.0 Attachments: THRIFT-1767_union_clear_value_experimental.patch, THRIFT-1767_unions_cant_have_required_fields.patch Required fields within unions should be treaten as optional, the union construct would not make much sense otherwise. -- 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
[jira] [Commented] (THRIFT-1772) Serialization does not check types of embedded structures.
[ https://issues.apache.org/jira/browse/THRIFT-1772?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13509987#comment-13509987 ] Henrique Mendonca commented on THRIFT-1772: --- Thanks Avi, Although it might reduce performance, I think we should consider also checking the object type. Do you want to send us a patch? Please see http://wiki.apache.org/thrift/HowToContribute Moreover, this probably affects the JS lib too, no sure about php and other weakly-typed languages... Can anyone help us? Cheers, Henrique Serialization does not check types of embedded structures. -- Key: THRIFT-1772 URL: https://issues.apache.org/jira/browse/THRIFT-1772 Project: Thrift Issue Type: Bug Components: Python - Library Affects Versions: 0.8, 0.9 Reporter: Avi Flamholz Consider the following struct definitions: struct Point { 1: required double x; 2: required double y; } struct Review { 1: required i32 rating; 2: optional string text; } struct Place { 1: required string name; 2: required Point location; 3: optional Review review; } If I create a Place object and set the location field to a Review, it will serialize no problem. place = Place(name=avi's place, location=Review(rating=1.0)) transportOut = TTransport.TMemoryBuffer() protocolOut = TBinaryProtocol.TBinaryProtocol(transportOut) place.write(protocolOut) serialized = transportOut.getvalue() This is confusing because if I set an i32 field to a string serialization does raise an error. -- 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
How should I implement multi-threaded clients and multi-threaded connections in C++?
Suppose I have a .thrift spec similar to the following: service Slow { void slowOperation(), void stopEverything(), } When my application launches, my code will establish a connection with the server, and hold on to that connection for the lifetime of the app. When a user launches a dialog box from this application, I want to call slowOperation(). If the user hits Cancel on the dialog, I want to run stopEverything(). stopEverything should cause slowOperation to stop what it is doing and complete. I have several problems with this setup right now. First, the codegenerated SlowClient class isn't thread safe. If multiple threads try to access it at the same time, bad things will happen. Second, as far as I can tell, the existing servers only process messages on one thread per connection. That means that even if I managed to send a stopEverything() call while a slowOperation() was in progress, the server wouldn't process that message until the slowOperation was complete. Does the C++ library / compiler currently have anything that can satisfy this requirement? I'm looking at some of the COB / Continuation OBject stuff, but it doesn't look like it makes the SlowClient thread safe (but maybe the AsyncChannel is supposed to be safe instead?). It also looks like there isn't an easy way to get this behavior for any given transport. I'm fine adding support for this kind of use case, but I want to make sure that nothing already exists that I've overlooked before I go down that path.
[jira] [Comment Edited] (THRIFT-1767) unions can't have required fields (Delphi)
[ https://issues.apache.org/jira/browse/THRIFT-1767?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13509975#comment-13509975 ] Jens Geyer edited comment on THRIFT-1767 at 12/4/12 9:27 PM: - I'm not so sure ... {code} Union { 1 : i32 one, 2 : i32 two = 3 } {code} would be ok. Only {code} Union { 1 : i32 one = 4, 2 : i32 two = 3 } {code} would produce something unexpected. Depending on the impl either more than one field would be set, or only the last value. But yes, I agree that a warning should be emitted. Sidenote: The Java impl indeed seems not to use the Defaults. was (Author: jensg): I'm not so sure. {code} Union { 1 : i32 one, 2 : i32 two = 3 } {code} Would be ok. Only {code} Union { 1 : i32 one = 4, 2 : i32 two = 3 } {code} would produce something unexpected. Depending on the impl either more than one field would be set, or only the last value. But yes, a warning should be emitted. unions can't have required fields (Delphi) -- Key: THRIFT-1767 URL: https://issues.apache.org/jira/browse/THRIFT-1767 Project: Thrift Issue Type: Bug Components: Delphi - Compiler Affects Versions: 0.8, 0.9 Reporter: Jens Geyer Assignee: Jens Geyer Fix For: 1.0 Attachments: THRIFT-1767_union_clear_value_experimental.patch, THRIFT-1767_unions_cant_have_required_fields.patch Required fields within unions should be treaten as optional, the union construct would not make much sense otherwise. -- 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
Re: How should I implement multi-threaded clients and multi-threaded connections in C++?
Hi Ben, just as an idea what I would be trying: 1. extract the implementation of slowOperation() and stopEverything() from the service into a dedicated class and make that one threadsafe. 2. if you need only one instance of this implementation class, make it a singleton or even a static class. if you want to have more than one in parallel, you need a (threadsafe) container for them. In the latter case go to step 3, otherwise continue with 4 3. Give each implemenation instance an ID and return this ID through an additional createOperation() method to the client. This ID must be passed in when slowOperation() or stopEverything() are called to locate the correct implementation instance. Since createOperation() creates a new implementation instance, don't forget that this instance must be deleted afterwards. 4. modify the service methods accordingly, they now just call the implementation and create/free the instances. 5. the client is now free to open another connection to the server to call stopEverything(). This call sets some kind of abort flag at the implementation instance identified through the ID. The flag ist tested repeatedly by slowOperation(). Could that work for you? JensG -Ursprüngliche Nachricht- From: Ben Craig Sent: Tuesday, December 4, 2012 10:20 PM To: dev@thrift.apache.org Subject: How should I implement multi-threaded clients and multi-threaded connections in C++? Suppose I have a .thrift spec similar to the following: service Slow { void slowOperation(), void stopEverything(), } When my application launches, my code will establish a connection with the server, and hold on to that connection for the lifetime of the app. When a user launches a dialog box from this application, I want to call slowOperation(). If the user hits Cancel on the dialog, I want to run stopEverything(). stopEverything should cause slowOperation to stop what it is doing and complete. I have several problems with this setup right now. First, the codegenerated SlowClient class isn't thread safe. If multiple threads try to access it at the same time, bad things will happen. Second, as far as I can tell, the existing servers only process messages on one thread per connection. That means that even if I managed to send a stopEverything() call while a slowOperation() was in progress, the server wouldn't process that message until the slowOperation was complete. Does the C++ library / compiler currently have anything that can satisfy this requirement? I'm looking at some of the COB / Continuation OBject stuff, but it doesn't look like it makes the SlowClient thread safe (but maybe the AsyncChannel is supposed to be safe instead?). It also looks like there isn't an easy way to get this behavior for any given transport. I'm fine adding support for this kind of use case, but I want to make sure that nothing already exists that I've overlooked before I go down that path.
[jira] [Updated] (THRIFT-1768) unions can't have required fields (Compiler)
[ https://issues.apache.org/jira/browse/THRIFT-1768?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jens Geyer updated THRIFT-1768: --- Attachment: (was: THRIFT-1768_unions_cant_have_required_fields_Compiler.patch) unions can't have required fields (Compiler) Key: THRIFT-1768 URL: https://issues.apache.org/jira/browse/THRIFT-1768 Project: Thrift Issue Type: Bug Components: Compiler (General) Affects Versions: 0.8, 0.9 Reporter: Jens Geyer Assignee: Jens Geyer Fix For: 1.0 Required fields within unions should issue a warning. First, required makes not much sense with unions. Next, most languages are already treating them as optional anyway, effectively ignoring the required attribute. -- 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
[jira] [Updated] (THRIFT-1768) unions can't have required fields (Compiler)
[ https://issues.apache.org/jira/browse/THRIFT-1768?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jens Geyer updated THRIFT-1768: --- Attachment: THRIFT-1768_unions_cant_have_required_fields_Compiler - rev1.patch Revised patch: * warning for required fields in unions * required field in unions are now forced to be optional * more than one default value within a union throws an error. I'm not sure about the Defaults anyway. I can't see an argument against having a default, except that Java as the reference implementation completely ignores them (without a warning, by the way) unions can't have required fields (Compiler) Key: THRIFT-1768 URL: https://issues.apache.org/jira/browse/THRIFT-1768 Project: Thrift Issue Type: Bug Components: Compiler (General) Affects Versions: 0.8, 0.9 Reporter: Jens Geyer Assignee: Jens Geyer Fix For: 1.0 Attachments: THRIFT-1768_unions_cant_have_required_fields_Compiler - rev1.patch Required fields within unions should issue a warning. First, required makes not much sense with unions. Next, most languages are already treating them as optional anyway, effectively ignoring the required attribute. -- 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
[jira] [Updated] (THRIFT-1767) unions can't have required fields (Delphi)
[ https://issues.apache.org/jira/browse/THRIFT-1767?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jens Geyer updated THRIFT-1767: --- Attachment: THRIFT-1767_unions_cant_have_required_fields_Delphi - rev2.patch Revised patch: * Important: This patch relies on THRIFT-1768 patch (rev1) being implemented! * removed most of the additional tests for is_union() introduced in the original patch, as they were no longer necessary if T_OPTIONAL is enforced for union fields. * slightly enhanced ClearUnionValues() method. unions can't have required fields (Delphi) -- Key: THRIFT-1767 URL: https://issues.apache.org/jira/browse/THRIFT-1767 Project: Thrift Issue Type: Bug Components: Delphi - Compiler Affects Versions: 0.8, 0.9 Reporter: Jens Geyer Assignee: Jens Geyer Fix For: 1.0 Attachments: THRIFT-1767_union_clear_value_experimental.patch, THRIFT-1767_unions_cant_have_required_fields_Delphi - rev2.patch, THRIFT-1767_unions_cant_have_required_fields.patch Required fields within unions should be treaten as optional, the union construct would not make much sense otherwise. -- 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
[jira] [Updated] (THRIFT-1767) unions can't have required fields (Delphi)
[ https://issues.apache.org/jira/browse/THRIFT-1767?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jens Geyer updated THRIFT-1767: --- Attachment: (was: THRIFT-1767_unions_cant_have_required_fields.patch) unions can't have required fields (Delphi) -- Key: THRIFT-1767 URL: https://issues.apache.org/jira/browse/THRIFT-1767 Project: Thrift Issue Type: Bug Components: Delphi - Compiler Affects Versions: 0.8, 0.9 Reporter: Jens Geyer Assignee: Jens Geyer Fix For: 1.0 Attachments: THRIFT-1767_union_clear_value_experimental.patch, THRIFT-1767_unions_cant_have_required_fields_Delphi - rev2.patch Required fields within unions should be treaten as optional, the union construct would not make much sense otherwise. -- 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
[jira] [Updated] (THRIFT-1769) unions can't have required fields (C++)
[ https://issues.apache.org/jira/browse/THRIFT-1769?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Jens Geyer updated THRIFT-1769: --- Patch Info: (was: Patch Available) Patch may need revision - postponed. unions can't have required fields (C++) --- Key: THRIFT-1769 URL: https://issues.apache.org/jira/browse/THRIFT-1769 Project: Thrift Issue Type: Bug Components: C++ - Compiler Affects Versions: 0.8, 0.9 Reporter: Jens Geyer Assignee: Jens Geyer Fix For: 1.0 Attachments: THRIFT-1769_unions_cant_have_required_fields_CPP.patch Required fields within unions should be treaten as optional, the union construct would not make much sense otherwise. -- 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
[jira] [Created] (THRIFT-1775) Java ignores default value(s) for unions
Jens Geyer created THRIFT-1775: -- Summary: Java ignores default value(s) for unions Key: THRIFT-1775 URL: https://issues.apache.org/jira/browse/THRIFT-1775 Project: Thrift Issue Type: Bug Components: Java - Compiler Reporter: Jens Geyer Given the following IDL, the expectation is that the generated code should initialize the one field with the default value accordingly. This is not the case, the value is silently ignored. {code} union TestCase { 1: string one 2: string two = I am your friendly default 3: string three } {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
Re: How should I implement multi-threaded clients and multi-threaded connections in C++?
Thanks for the reply Jens. Here is what I think you are suggesting: service SlowRunner { i32 createOperation(), void slowOperation(1: i32 instanceID), } service SlowStopper { void stopEverything(1: i32 instanceID), } So with my interface, a client would call SlowClient::slowOperation() in the main thread, and SlowClient::stopEverything() on the stop thread, both on the same SlowClient instance. With this new interface, you are suggesting that the main thread call SlowRunnerClient::createOperation(), then call SlowRunnerClient::slowOperation(). If I need to stop things, then the stop thread will need to get the value of the instanceID, then call SlowStopper::stopEverything() on a SlowStopper instance. This solves the problem as specified in my post, at the cost of extra connections. In reality, it doesn't solve my problem. My desired interface is actually more like this: service GenericService { string doSomething(1: string cmd), } One of the commands would be doSlowOperation and another would be stopEverything. There would also be per connection state, to service hypothetical commands like addToQueue and removeFromQueue. So even with this, it would be possible to add a bunch of instance ids to the service, and force the client and server implementations to create lots of connections and session tracking. However, that seems like it is adding one session tracking system (instanceIDs) on top of a different session tracking system (connections). I'm fine if Thrift doesn't currently support such a use case, and I will gladly develop and contribute the support. I have several uses for this threading support in my company. I just want to make sure that I'm not duplicating the work that already exists in some corner of Thrift I haven't become familiar with yet. From: Jens Geyer jensge...@hotmail.com To: dev@thrift.apache.org, Date: 12/04/2012 04:00 PM Subject:Re: How should I implement multi-threaded clients and multi-threaded connections in C++? Hi Ben, just as an idea what I would be trying: 1. extract the implementation of slowOperation() and stopEverything() from the service into a dedicated class and make that one threadsafe. 2. if you need only one instance of this implementation class, make it a singleton or even a static class. if you want to have more than one in parallel, you need a (threadsafe) container for them. In the latter case go to step 3, otherwise continue with 4 3. Give each implemenation instance an ID and return this ID through an additional createOperation() method to the client. This ID must be passed in when slowOperation() or stopEverything() are called to locate the correct implementation instance. Since createOperation() creates a new implementation instance, don't forget that this instance must be deleted afterwards. 4. modify the service methods accordingly, they now just call the implementation and create/free the instances. 5. the client is now free to open another connection to the server to call stopEverything(). This call sets some kind of abort flag at the implementation instance identified through the ID. The flag ist tested repeatedly by slowOperation(). Could that work for you? JensG -Ursprüngliche Nachricht- From: Ben Craig Sent: Tuesday, December 4, 2012 10:20 PM To: dev@thrift.apache.org Subject: How should I implement multi-threaded clients and multi-threaded connections in C++? Suppose I have a .thrift spec similar to the following: service Slow { void slowOperation(), void stopEverything(), } When my application launches, my code will establish a connection with the server, and hold on to that connection for the lifetime of the app. When a user launches a dialog box from this application, I want to call slowOperation(). If the user hits Cancel on the dialog, I want to run stopEverything(). stopEverything should cause slowOperation to stop what it is doing and complete. I have several problems with this setup right now. First, the codegenerated SlowClient class isn't thread safe. If multiple threads try to access it at the same time, bad things will happen. Second, as far as I can tell, the existing servers only process messages on one thread per connection. That means that even if I managed to send a stopEverything() call while a slowOperation() was in progress, the server wouldn't process that message until the slowOperation was complete. Does the C++ library / compiler currently have anything that can satisfy this requirement? I'm looking at some of the COB / Continuation OBject stuff, but it doesn't look like it makes the SlowClient thread safe (but maybe the AsyncChannel is supposed to be safe instead?). It also looks like there isn't an easy way to get this behavior for any given transport. I'm fine adding support for this kind of use case, but I want to make sure that nothing already exists that I've overlooked before I go down that path.
[jira] [Commented] (THRIFT-1772) Serialization does not check types of embedded structures.
[ https://issues.apache.org/jira/browse/THRIFT-1772?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13510165#comment-13510165 ] Avi Flamholz commented on THRIFT-1772: -- Henrique -- I have a working and tested patch, but it doesn't fix the fastbinary protocol. I don't really know how to fix that. Should I submit the patch and then you guys can fix fastbinary? Avi Serialization does not check types of embedded structures. -- Key: THRIFT-1772 URL: https://issues.apache.org/jira/browse/THRIFT-1772 Project: Thrift Issue Type: Bug Components: Python - Library Affects Versions: 0.8, 0.9 Reporter: Avi Flamholz Consider the following struct definitions: struct Point { 1: required double x; 2: required double y; } struct Review { 1: required i32 rating; 2: optional string text; } struct Place { 1: required string name; 2: required Point location; 3: optional Review review; } If I create a Place object and set the location field to a Review, it will serialize no problem. place = Place(name=avi's place, location=Review(rating=1.0)) transportOut = TTransport.TMemoryBuffer() protocolOut = TBinaryProtocol.TBinaryProtocol(transportOut) place.write(protocolOut) serialized = transportOut.getvalue() This is confusing because if I set an i32 field to a string serialization does raise an error. -- 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
Re: How should I implement multi-threaded clients and multi-threaded connections in C++?
Hi Ben, Here is what I think you are suggesting: service SlowRunner { ... } service SlowStopper { ... } I don't think that two different services are really necessary. In fact, I would not even recommend it for your particular case. One of the commands would be doSlowOperation and another would be stopEverything. There would also be per connection state, to service hypothetical commands like addToQueue and removeFromQueue. You didn't say that in your use case description. You didn't even say that slowOperation() must be a blocking operation, I just assumed that from your description. Let's just assume, that slowOperation() could be executed asyncronously without problems. Then the client would not even need to wait for it leaving the connection open all the time! The longer I think about it, the more I would like to suggest to sketch the entire scenario more closely to answer all those questions popping up. How is the queue involved with slowOperation()? Maybe slowOperation() takes work items from a queue and processes them? If yes, why is it necessary to call slowOperation() at all? Couldn't processing be triggered automatically by addToQueue()? And by the way, how does the client keep track of the status of the running operations, e.g. to visualize their progress in the UI? However, that seems like it is adding one session tracking system (instanceIDs) on top of a different session tracking system (connections). There are cases where open connections are a much more scarce resource than memory or disk space is, so this is not necessarily such a bad idea. One should carefully differentiate the sessions defined by some internal state from sessions defined by a connection. The connection is just the entry point used to access the internal state, but it does not necessarily have the same life time as that state. The lifetime of the internal state can easily span hundrets or thousands of established and closed connections, all used to access the very same internal state data. And that's the first and foremost reason why I would separate service implementation from implementation of the logic made accessible by means of the service: Separation of Concerns. My desired interface is actually more like this: service GenericService { string doSomething(1: string cmd), } So you are adding one interface (your command line syntax) on top of a different interface (Thrift)?;-) I'm fine if Thrift doesn't currently support such a use case, and I will gladly develop and contribute the support. I have several uses for this threading support in my company. I just want to make sure that I'm not duplicating the work that already exists in some corner of Thrift I haven't become familiar with yet. Agree. Maybe someone with more expertise regarding that particular area of the code joins the discussion, but I'm sure any contribution is welcome. Best regards, JensG -Ursprüngliche Nachricht- From: Ben Craig Sent: Tuesday, December 4, 2012 11:51 PM To: dev@thrift.apache.org Subject: Re: How should I implement multi-threaded clients and multi-threaded connections in C++? Thanks for the reply Jens. Here is what I think you are suggesting: service SlowRunner { i32 createOperation(), void slowOperation(1: i32 instanceID), } service SlowStopper { void stopEverything(1: i32 instanceID), } So with my interface, a client would call SlowClient::slowOperation() in the main thread, and SlowClient::stopEverything() on the stop thread, both on the same SlowClient instance. With this new interface, you are suggesting that the main thread call SlowRunnerClient::createOperation(), then call SlowRunnerClient::slowOperation(). If I need to stop things, then the stop thread will need to get the value of the instanceID, then call SlowStopper::stopEverything() on a SlowStopper instance. This solves the problem as specified in my post, at the cost of extra connections. In reality, it doesn't solve my problem. My desired interface is actually more like this: service GenericService { string doSomething(1: string cmd), } One of the commands would be doSlowOperation and another would be stopEverything. There would also be per connection state, to service hypothetical commands like addToQueue and removeFromQueue. So even with this, it would be possible to add a bunch of instance ids to the service, and force the client and server implementations to create lots of connections and session tracking. However, that seems like it is adding one session tracking system (instanceIDs) on top of a different session tracking system (connections). I'm fine if Thrift doesn't currently support such a use case, and I will gladly develop and contribute the support. I have several uses for this threading support in my company. I just want to make sure that I'm not duplicating the work that already exists in some corner of Thrift I haven't become familiar with yet. From:
Re: How should I implement multi-threaded clients and multi-threaded connections in C++?
Jens, Here is what I think you are suggesting: service SlowRunner { ... } service SlowStopper { ... } I don't think that two different services are really necessary. In fact, I would not even recommend it for your particular case. Yep, you're right. Two services don't make much sense there. Put the stopEverything function in the SlowRunner service and I think stuff makes sense again. You didn't say that in your use case description. I apologize. I'm trying to be helpful without pre-announcing anything from my company. I think I've come up with some better explanations though. Right now, I'm working with several cases where something that is currently in-process needs to move out-of-process. The in-process code has a well defined API, and I want to keep something extremely similar to that API where possible. The in-process code often has sessions and objects that can be operated on from multiple threads in a thread safe way. Some of these operations are I/O operations with client specified time-outs. These APIs provide a way to cancel a session's I/O from another thread. Most (but not all) of these use cases already have session objects that would be a very reasonable place to put a thrift client. The servers are likely to be resource constrained though. If at all possible, I want to maintain the assumptions that calling code had before. Basically, code that wasn't serialized in-process before shouldn't be serialized in the out-of-process case. So I'll argue that I got separation of concerns down first :). The API concern was solved before I even started researching Thrift. I just want a server (and a client) that will guard the internal transport appropriately so that one blocked request doesn't stall other, newer requests on the same transport. My desired interface is actually more like this: service GenericService { string doSomething(1: string cmd), } So you are adding one interface (your command line syntax) on top of a different interface (Thrift)?;-) Yep, Thrift is my golden hammer of choice right now. ( http://en.wikipedia.org/wiki/Law_of_the_instrument) It seems a lot easier to use the servers, clients, and transports as provided in Thrift than to re-roll all of that, even if the end result might be a little simpler and smaller.