[review] Review of Boost.Exception
Hi Everyone, Below is my review of the proposed Boost.Exception library. * What is your evaluation of the design? I personally like that the exception library is implemented in just a couple of header files, without having to drag in a lot of other libraries. It is succinctly implemented, though I have some reservations about some design decisions: get_error_info(T const &) returns a pointer instead of a const copy of the actual error_info it gets. This can be a potential headache to deal with especially in examining the error_info associated with the thrown exception. A proposed change would be to have get_error_info(T const &) return a const copy of the data, while get_error_info_ptr(T const &) would return a pointer as in the current version. Although it might also take quite some effort to remove the runtime polymorphic behavior which relies on shared_ptr<>'s and virtual tables with info_base, perhaps an expression templates-based approach to building exceptions that derive from a common base could mitigate these (theoretical) inefficiencies introduced with runtime polymorphic behavior? A linear inheritance encapsulating the different information using something like Boost.Fusion's or Boost.MPL's `fold` concept to create an exception type using expression templates might work. Then I haven't tried this myself, so it might be worth considering in the future. My concern with the use of shared_ptr<>'s is that in cases where an exception pertaining to "out of memory" conditions would arise, throwing a boost::exception and then having to allocate more memory just to encapsulate additional information would lead to undesirable termination. So keeping exception data statically allocated on the stack instead of being in the heap might be a good alternative. This is just my thinking though I don't have enough data to back this up. * What is your evaluation of the implementation? It's very cleanly implemented, and very much readable. * What is your evaluation of the documentation? The documentation is very well written although an alternative example aside from the case with file IO would be greatly appreciated. That said, the example given is sufficient enough to make a case for the utility of the library. * What is your evaluation of the potential usefulness of the library? In cases where minimal memory footprints are required, the current implementation introduces a lot of unnecessary allocations on the heap when additional information is being encapsulated into the exception. An expression templates-based approach as described above (though untested) should be able to mitigate this consequence. In all other cases where efficiency and performance are not critical, I can see this exception library to be very useful. * Did you try to use the library? With what compiler? Did you have any problems? Not yet, though I see myself using this in a project really soon. * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I went into the implementation details, and the conceptual design based on the source code. I did a quick read of the documentation, which didn't seem to change since the first time this library has been introduced. * Are you knowledgeable about the problem domain? I have been writing ad-hoc exception hierarchies based on std::exception for quite a while already, and I can say that this implementation of exceptions is a welcome development. * Do you think the library should be accepted as a Boost library? Be sure to say this explicitly so that your other comments don't obscure your overall opinion. I think yes, it should be accepted as a Boost library. I can see it getting a lot of improvements in terms of implementation though, but being in the Boost C++ Library will allow more people to be able to propose changes and eventually extend this library in many different ways. -- Dean Michael C. Berris Software Engineer, Friendster, Inc. [http://cplusplus-soup.blogspot.com/] [mikhailberis@gmail.com] [+63 928 7291459] [+1 408 4049523]
get_error_info(T const &) returns a pointer instead of a const copy of the actual error_info it gets. This can be a potential headache to deal with especially in examining the error_info associated with the thrown exception. A proposed change would be to have get_error_info(T const &) return a const copy of the data, while get_error_info_ptr(T const &) would return a pointer as in the current version.
I suppose the behavior you propose for get_error_info is to throw an exception if the requested data is not available in the exception. But think about the situation you're describing: you catch an exception, and now you want to query it for error_info -- what good does it do for that context to throw an exception? Sure, there are use cases when you know an error_info must be present in the exception or you have a bug, but in that case it makes more sense to assert on get_error_info.
Although it might also take quite some effort to remove the runtime polymorphic behavior which relies on shared_ptr<>'s and virtual tables with info_base <snipped>
I never had any performance-related issues with this library, and I've been using it for more than a year now. Until we have evidence that we need to optimize, I would prefer to keep the implementation as simple as possible.
My concern with the use of shared_ptr<>'s is that in cases where an exception pertaining to "out of memory" conditions would arise, throwing a boost::exception and then having to allocate more memory just to encapsulate additional information would lead to undesirable termination.
Could you elaborate a bit more? Can you come up with a theoretical example that demonstrates your concerns? Thanks for you feedback! Emil Dotchevski
Hi Emil! On 9/30/07, Emil Dotchevski <emildotchevski@gmail.com> wrote:
get_error_info(T const &) returns a pointer instead of a const copy of the actual error_info it gets. This can be a potential headache to deal with especially in examining the error_info associated with the thrown exception. A proposed change would be to have get_error_info(T const &) return a const copy of the data, while get_error_info_ptr(T const &) would return a pointer as in the current version.
I suppose the behavior you propose for get_error_info is to throw an exception if the requested data is not available in the exception. But think about the situation you're describing: you catch an exception, and now you want to query it for error_info -- what good does it do for that context to throw an exception?
Sure, there are use cases when you know an error_info must be present in the exception or you have a bug, but in that case it makes more sense to assert on get_error_info.
Far from it. I'm thinking get_error_info(T const &) should not compile if the error_info being requested has not been passed to the exception. This would only be possible if the exception type itself is composed using expression templates and its type information is somehow reflected during runtime -- or it might be impossible given that the exception is referred to via an abstract base `boost::error`. Maybe an encapsulated tuple that "grows" along the exception propagation might work though I'm not entirely sure about that as well. At any rate, perhaps using something like Boost.Optional as the return value (or an additional parameter to get_error_info) mitigates the problem you describe. Somehow, for me the following reads a bit more coherent if not safer: try { ... } catch (boost::exception & e) { boost::optional<int> errno = boost::get_error_info<tag::errno>(e); if (errno) { // means errno is set cerr << *errno; } else { // means errno is not set, then set it e << boost::error_info<tag::errno>(-1); }; throw; // propagate exception }; This way, I cannot easily circumvent the pointer returned and mangle it perhaps to further mess things up. Not that I'm saying anybody would do this, but since someone could do it, then it's easier to have users shoot themselves in the foot instead of providing an interface that's considerably safer than pointers. Not that there's anything bad with pointers, it's just too easy to get them wrong that it's usually a good idea to stay away from them as much as you can. That's what I think at least.
Although it might also take quite some effort to remove the runtime polymorphic behavior which relies on shared_ptr<>'s and virtual tables with info_base <snipped>
I never had any performance-related issues with this library, and I've been using it for more than a year now. Until we have evidence that we need to optimize, I would prefer to keep the implementation as simple as possible.
It's good to know that you haven't had any performance-related issues with this library. I'll take your word for it. However, a shared_ptr<> implies allocating something on the heap. Anytime you make an (arguably) unnecessary memory allocation in the heap, in my book is a performance affecting issue. We can go on and argue about long jumps, cache misses, and memory locality issues, but this would be moot since the goal of the library is primarily to provide a functionality that seems to require the heap allocation *by design*. So I agree, unless there's evidence that shows it's a performance issue (having shared_ptr<>'s and runtime polymorphic behavior in exceptions) then I can't convince you that shared_ptr<>'s might be needless and that there might be a better way of doing this. I'll also say I wouldn't even try. ;-)
My concern with the use of shared_ptr<>'s is that in cases where an exception pertaining to "out of memory" conditions would arise, throwing a boost::exception and then having to allocate more memory just to encapsulate additional information would lead to undesirable termination.
Could you elaborate a bit more? Can you come up with a theoretical example that demonstrates your concerns?
Theoretically, (or I think even in reality) a call to the (default) new operator may trigger a bad_alloc exception. This exception may be caught, and somewhere along the way code decides to encapsulate the information in that bad_alloc exception into a boost::exception -- and chooses to say where that exception happened, as a tag in the propagated boost::exception. So having additional heap allocations when an out of memory exception is already thrown doesn't make a lot of sense mainly because you've already run out of memory that's why the exception is thrown in the first place. Although fixing bad programming practice is not within the design goals of the proposed Exception library, making it harder for users to use it in a wrong manner would be a nice goal IMHO.
Thanks for you feedback!
You're welcome. :) -- Dean Michael C. Berris Software Engineer, Friendster, Inc. [http://cplusplus-soup.blogspot.com/] [mikhailberis@gmail.com] [+63 928 7291459] [+1 408 4049523]
Dean Michael Berris wrote:
Hi Emil!
Far from it. I'm thinking get_error_info(T const &) should not compile if the error_info being requested has not been passed to the exception. This would only be possible if the exception type itself is composed using expression templates and its type information is somehow reflected during runtime -- or it might be impossible given that the exception is referred to via an abstract base `boost::error`. Maybe an encapsulated tuple that "grows" along the exception propagation might work though I'm not entirely sure about that as well.
This is completely impossible. Full static type information would have to be available in order to compose a new data item into an existing exception, which means that a site which just wants to add information would have to catch the exception by its _full expression template type_. Given that a site might not even _know_ what exceptions might pass through it (if it's passed a Boost.Function object and calls it, for example), that's not even possible, not to mention that it would be completely unusuable if it _were_ possible. Sebastian Redl
On 9/30/07, Sebastian Redl <sebastian.redl@getdesigned.at> wrote:
Dean Michael Berris wrote:
Hi Emil!
Far from it. I'm thinking get_error_info(T const &) should not compile if the error_info being requested has not been passed to the exception. This would only be possible if the exception type itself is composed using expression templates and its type information is somehow reflected during runtime -- or it might be impossible given that the exception is referred to via an abstract base `boost::error`. Maybe an encapsulated tuple that "grows" along the exception propagation might work though I'm not entirely sure about that as well.
This is completely impossible. Full static type information would have to be available in order to compose a new data item into an existing exception, which means that a site which just wants to add information would have to catch the exception by its _full expression template type_. Given that a site might not even _know_ what exceptions might pass through it (if it's passed a Boost.Function object and calls it, for example), that's not even possible, not to mention that it would be completely unusuable if it _were_ possible.
This confirms my suspicions. Sorry for the mental fart earlier. So while I was not 100% sure it was impossible, now with this explanation I'm convinced it's impossible. That doesn't take away the feasibility of an expression template in constructing an exception that 1) doesn't require the use of shared_ptr<>'s to store additional tagged information 2) derives from a single base `boost::exception` and 3) returns a Boost.Optional value instead of a const data * to the value associated with an exception_info tag. Right? -- Dean Michael C. Berris Software Engineer, Friendster, Inc. [http://cplusplus-soup.blogspot.com/] [mikhailberis@gmail.com] [+63 928 7291459] [+1 408 4049523]
<snip> Sure, there are use cases when you know an error_info must be present in the exception or you have a bug, but in that case it makes more sense to assert on get_error_info.
Far from it. I'm thinking get_error_info(T const &) should not compile if the error_info being requested has not been passed to the exception.
I don't think this is possible. Consider that a main design goal of boost::exception is to be able to do this: try { ---something--- } catch( boost::exception & x ) { x << error_info<my_info>(....); throw; } In general, when we finally catch the exception object, it may have my_info or not.
At any rate, perhaps using something like Boost.Optional as the return value (or an additional parameter to get_error_info) mitigates the problem you describe. Somehow, for me the following reads a bit more coherent if not safer:
try { ... } catch (boost::exception & e) { boost::optional<int> errno = boost::get_error_info<tag::errno>(e); if (errno) { // means errno is set cerr << *errno; } else { // means errno is not set, then set it e << boost::error_info<tag::errno>(-1); }; throw; // propagate exception };
Would you agree that this is simpler and easier to read?: try { ... } catch (boost::exception & e) { if( int const * ec = boost::get_error_info<tag_errno>(e) ) { cerr << *ec; } else { e << boost::error_info<tag::errno>(-1); }; throw; // propagate exception };
<snip> It's good to know that you haven't had any performance-related issues with this library. I'll take your word for it.
However, a shared_ptr<> implies allocating something on the heap.
Anytime you make an (arguably) unnecessary memory allocation in the heap, in my book is a performance affecting issue. We can go on and argue about long jumps, cache misses, and memory locality issues, but this would be moot since the goal of the library is primarily to provide a functionality that seems to require the heap allocation *by design*.
In the particular case of Boost Exception, any speed overhead it adds should be compared to the overhead of throwing an exception. You have stack unwinding and destructor calls, etc. anyway. I don't think that optimizing Boost Exception for speed makes any sense. Optimizing the memory allocations made by Boost Exception when info is added to exception objects can not be dismissed as easily, but I would not call those allocations unnecessary. :) Consider that all of these allocations happen immediately before or during stack unwinding, and at that time the rest of the code doesn't usually allocate memory. When the exception is finally handled, all this memory is reclaimed, and assuming that no other memory was allocated during the stack unwinding, we don't get fragmentation either.
My concern with the use of shared_ptr<>'s is that in cases where an exception pertaining to "out of memory" conditions would arise, throwing a boost::exception and then having to allocate more memory just to encapsulate additional information would lead to undesirable termination.
Could you elaborate a bit more? Can you come up with a theoretical example that demonstrates your concerns?
Theoretically, (or I think even in reality) a call to the (default) new operator may trigger a bad_alloc exception. This exception may be caught, and somewhere along the way code decides to encapsulate the information in that bad_alloc exception into a boost::exception -- and chooses to say where that exception happened, as a tag in the propagated boost::exception.
I guess I should have been more specific, could you elaborate how using boost::exception would "lead to undesirable termination"? Emil Dotchevski
On 9/30/07, Emil Dotchevski <emil@revergestudios.com> wrote:
[snip]
In the particular case of Boost Exception, any speed overhead it adds should be compared to the overhead of throwing an exception. You have stack unwinding and destructor calls, etc. anyway. I don't think that optimizing Boost Exception for speed makes any sense.
I agree with this.
Optimizing the memory allocations made by Boost Exception when info is added to exception objects can not be dismissed as easily, but I would not call those allocations unnecessary. :)
But I believe that we should be able to allow not making any heap allocation within some measure. Using a custom pre-allocated heap should make *some* information to be held without any allocation. Types that use heap on construction/copy-construction we don't have much choice anyway. It could be disabled/enabled and the size could be customized.
Consider that all of these allocations happen immediately before or during stack unwinding, and at that time the rest of the code doesn't usually allocate memory. When the exception is finally handled, all this memory is reclaimed, and assuming that no other memory was allocated during the stack unwinding, we don't get fragmentation either.
My biggest concern is bad_alloc being thrown and the exception being replaced. It might not be as bad for most cases, but may be critical in others. [snip]
Emil Dotchevski
-- Felipe Magno de Almeida
<snip> Optimizing the memory allocations made by Boost Exception when info is added to exception objects can not be dismissed as easily, but I would not call those allocations unnecessary. :)
But I believe that we should be able to allow not making any heap allocation within some measure.
In theory the allocator support in the shared_ptr inside the boost::exception class can be used to abstract the actual allocations. While I'm not opposed to that in principle, I can't think of many environments that would welcome exceptions but not welcome memory allocations.
My biggest concern is bad_alloc being thrown and the exception being replaced. It might not be as bad for most cases, but may be critical in others.
Suppose we catch a bad_alloc, then (for reasons that are hard for me to imagine) we throw something_else, which derives from boost::exception, and we attempt to stuff it with all kinds of info. The two possible outcomes are, it either works fine, or we get another bad_alloc instead of something_else. I don't see how this could cause any problems (any more than the original bad_alloc would). Emil Dotchevski
On 9/30/07, Emil Dotchevski <emil@revergestudios.com> wrote:
<snip>
[snip]
My biggest concern is bad_alloc being thrown and the exception being replaced. It might not be as bad for most cases, but may be critical in others.
Suppose we catch a bad_alloc, then (for reasons that are hard for me to imagine) we throw something_else, which derives from boost::exception, and we attempt to stuff it with all kinds of info. The two possible outcomes are, it either works fine, or we get another bad_alloc instead of something_else.
I don't see how this could cause any problems (any more than the original bad_alloc would).
I'm actually trying to avoid the possibility of bad_alloc being throw by not allocating anything.
Emil Dotchevski
-- Felipe Magno de Almeida
On 9/30/07, Emil Dotchevski <emil@revergestudios.com> wrote:
I'm actually trying to avoid the possibility of bad_alloc being throw by not allocating anything.
How?
If all allocations from boost.exception could be made from a custom allocator, it should be possible. Or am I missing something? Regards, -- Felipe Magno de Almeida
I'm actually trying to avoid the possibility of bad_alloc being throw by not allocating anything.
How?
If all allocations from boost.exception could be made from a custom allocator, it should be possible. Or am I missing something?
Ah, I got confused by your previous points which seemed to indicate that you wanted to somehow avoid dynamic polymorphism -- and thus the need for memory allocation -- altogether (which is not possible.) Other boost components, such as boost::function and boost::shared_ptr, have an allocator parameter, so I'm assuming you're proposing something similar. We can add a template constructor to boost::exception that can be used to supply a custom allocator. I'd rather not do this, but (I think) it can be implemented in a way that wouldn't have any negative effects on users who don't want to use the custom allocator support, so if there's demand -- why not. Emil Dotchevski
On 9/30/07, Emil Dotchevski <emil@revergestudios.com> wrote:
<snip> Sure, there are use cases when you know an error_info must be present in the exception or you have a bug, but in that case it makes more sense to assert on get_error_info.
Far from it. I'm thinking get_error_info(T const &) should not compile if the error_info being requested has not been passed to the exception.
I don't think this is possible. Consider that a main design goal of boost::exception is to be able to do this:
try { ---something--- } catch( boost::exception & x ) { x << error_info<my_info>(....); throw; }
In general, when we finally catch the exception object, it may have my_info or not.
True, I'm sorry about the mental fart about it not compiling. Sebastian has already confirmed my suspicions and I don't think it would be practical to do this either.
At any rate, perhaps using something like Boost.Optional as the return value (or an additional parameter to get_error_info) mitigates the problem you describe. Somehow, for me the following reads a bit more coherent if not safer:
try { ... } catch (boost::exception & e) { boost::optional<int> errno = boost::get_error_info<tag::errno>(e); if (errno) { // means errno is set cerr << *errno; } else { // means errno is not set, then set it e << boost::error_info<tag::errno>(-1); }; throw; // propagate exception };
Would you agree that this is simpler and easier to read?:
try { ... } catch (boost::exception & e) { if( int const * ec = boost::get_error_info<tag_errno>(e) ) { cerr << *ec; } else { e << boost::error_info<tag::errno>(-1); }; throw; // propagate exception };
Personally, I wouldn't agree that this is simpler because of personal dislike for pointers, and how they can be mangled inadvertently. Consider the bug that can be introduced by pointer semantics: try { ... } catch (boost::exception & e) { if ( int const * ec = boost::get_error_info<tag::errno>(e) ) { cerr << ec + 1; } else { e << boost::error_info<tag::errno>(-1); }; throw; // propagate exception }; Consider having a boost::optional<int> in place of `int const * ec`, you don't run into this problem because adding 1 to an optional<int> won't compile. FWIW, boost::optional<int> ec can also be put inside the if condition so that it's available only inside the body of the if implementation. This also won't expose a memory address which someone can mangle with, not that it would make much sense doing that -- the fact that it's possible makes it inherently dangerous.
<snip> It's good to know that you haven't had any performance-related issues with this library. I'll take your word for it.
However, a shared_ptr<> implies allocating something on the heap.
Anytime you make an (arguably) unnecessary memory allocation in the heap, in my book is a performance affecting issue. We can go on and argue about long jumps, cache misses, and memory locality issues, but this would be moot since the goal of the library is primarily to provide a functionality that seems to require the heap allocation *by design*.
In the particular case of Boost Exception, any speed overhead it adds should be compared to the overhead of throwing an exception. You have stack unwinding and destructor calls, etc. anyway. I don't think that optimizing Boost Exception for speed makes any sense.
Speed not entirely, true. Memory efficiency and memory-related performance effects can arguably be reduced -- it's just a matter of how much effort you want to put or not put into it, for admittedly very little gain.
Optimizing the memory allocations made by Boost Exception when info is added to exception objects can not be dismissed as easily, but I would not call those allocations unnecessary. :)
Yes. What I meant though is that there should be a way to remove if not minimize these heap allocations into stack-allocated values encapsulated in the exception. How this can be done is still hazy to me, but I'm thinking it may have something to do with encapsulating the exception into another wrapping exception which is thrown instead of just letting the exception propagate in the natural scheme of things. This may be contrary to the original design goals, so I'm not sure how attractive that option is to you. :)
Consider that all of these allocations happen immediately before or during stack unwinding, and at that time the rest of the code doesn't usually allocate memory. When the exception is finally handled, all this memory is reclaimed, and assuming that no other memory was allocated during the stack unwinding, we don't get fragmentation either.
I'm not sure I understand this, but without using a object pool implementation like Boost.Pool that guarantees that areas in memory allocated are contiguous or at least minimizes the risk of fragmentation, I don't think it's safe to assume you don't get fragmentation during stack unwinding and/or allocating heap space within catch blocks.
My concern with the use of shared_ptr<>'s is that in cases where an exception pertaining to "out of memory" conditions would arise, throwing a boost::exception and then having to allocate more memory just to encapsulate additional information would lead to undesirable termination.
Could you elaborate a bit more? Can you come up with a theoretical example that demonstrates your concerns?
Theoretically, (or I think even in reality) a call to the (default) new operator may trigger a bad_alloc exception. This exception may be caught, and somewhere along the way code decides to encapsulate the information in that bad_alloc exception into a boost::exception -- and chooses to say where that exception happened, as a tag in the propagated boost::exception.
I guess I should have been more specific, could you elaborate how using boost::exception would "lead to undesirable termination"?
try { try { ... } catch (std::bad_alloc & e) { throw boost::exception() << error_info<tag::long_double>(0xFF); }; ... } catch (boost::exception & e) { e << error_info<tag::long_double_still>(0xFF); throw; }; Unless you're catching bad_alloc explicitly outside of the outer try-catch block, and unless bad_alloc derives from boost::exception, then the possibility of throwing a bad_alloc while adding error_info<tag::long_double_still>(0xFF) exists because you can run out of memory while allocating from the heap. Without a handler for bad_alloc outside the catch block, you're bound to get termination. Admittedly this example is contrived, but may be really hard to trace once the shared_ptr<>'s allocated when adding error_info objects decides to throw a bad_alloc exception. -- Dean Michael C. Berris Software Engineer, Friendster, Inc. [http://cplusplus-soup.blogspot.com/] [mikhailberis@gmail.com] [+63 928 7291459] [+1 408 4049523]
try { try { ... } catch (std::bad_alloc & e) { throw boost::exception() << error_info<tag::long_double>(0xFF); }; ... } catch (boost::exception & e) { e << error_info<tag::long_double_still>(0xFF); throw; };
Do I understand correctly that you are worried about a bad_alloc propagating up the stack, after: 1) you catch bad_alloc 2) you stuff something in a boost exception, which allocates memory successfully 3) you catch that exception after some stack unwinding (which presumably frees more memory) and then you attempt to stuff some more info in it Emil Dotchevski P.S. For the sake of clarity, I should say that you can not throw a boost::exception: it's an abstract base class.
On 9/30/07, Emil Dotchevski <emil@revergestudios.com> wrote:
try { try { ... } catch (std::bad_alloc & e) { throw boost::exception() << error_info<tag::long_double>(0xFF); }; ... } catch (boost::exception & e) { e << error_info<tag::long_double_still>(0xFF); throw; };
Do I understand correctly that you are worried about a bad_alloc propagating up the stack, after:
1) you catch bad_alloc
2) you stuff something in a boost exception, which allocates memory successfully
3) you catch that exception after some stack unwinding (which presumably frees more memory) and then you attempt to stuff some more info in it
Yes.
Emil Dotchevski
P.S. For the sake of clarity, I should say that you can not throw a boost::exception: it's an abstract base class.
Sorry about that, but the idea was to throw an exception and add more stuff into it as the stack unwinds and the exception propagates. -- Dean Michael C. Berris Software Engineer, Friendster, Inc. [http://cplusplus-soup.blogspot.com/] [mikhailberis@gmail.com] [+63 928 7291459] [+1 408 4049523]
Emil Dotchevski wrote:
In the particular case of Boost Exception, any speed overhead it adds should be compared to the overhead of throwing an exception. You have stack unwinding and destructor calls, etc. anyway. I don't think that optimizing Boost Exception for speed makes any sense.
Optimizing the memory allocations made by Boost Exception when info is added to exception objects can not be dismissed as easily, but I would not call those allocations unnecessary. :)
There are certainly avoidable ones: * intrusive::map<K,V> (or using intrusive::rb_tree directly to avoid dependencies) can replace std::map<K,shared_ptr<V> > to save one allocation per object. Taking it further by applying Tom Brinkman's suggestion you can have one allocation per custom_exception (see previous post), as those objects can be grouped. It takes a few lines of code to manage the deletion, though. * The shared pointer for pImpl could as well be an intrusive one saving another allocation per throw (and as exceptions are not accessed concurrently gets rid of the synchronization as well). Regards, Tobias Schwinger - Review Manager -
participants (6)
-
Dean Michael Berris -
Emil Dotchevski -
Emil Dotchevski -
Felipe Magno de Almeida -
Sebastian Redl -
Tobias Schwinger