On 24/08/05, Michael Glassford <glassfordm@hotmail.com> wrote:

...

Slawomir Lisznianski wrote: Hello,

I find the catch(...) clause in the thread_proxy function (thread.cpp) causing more harm than good.

Does anyone have any comments on this? Support or objections?

I don't think it is cut and dry. It is a case of consistent behaviour versus implementation defined behaviour. In the past I've found it suited myself too to get rid of the catch all clause so I'd probably come down in the get rid of it and leave it implementation defined. Exception safety across threads is a point of contentious design and best left to the user to deal with it by specific design I think. $0.02 matt. matthurd@acm.org

On 25/08/05, John Maddock <john@johnmaddock.co.uk> wrote: <snip> I think I'd come down on the other side: this isn't a minor change, it's a complete change of semantics and we should be very careful about such a change. With most of the compilers I use an uncaught exception thrown from a user thread will terminate the application, which may be what you want, or it may not: but it sure is drastic! You can always trap uncaught exceptions yourself and call abort (or whatever) if that's what you want. The debugger I use will also let you trap all thrown exceptions if that's what you want, whether they all do that I don't know.

Also 2c worth,

After thinking about it some more, I'd still much rather see a screaming application halt than the false sense of security suggested by silent failure. Yes, it is a big change, and needs more comment. As you suggest, you may catch anything in your own handler. I worry that silent collection of exceptions will allow too many subtle errors to slip through, especially in the land of concurrency. It has happenned to me that I've let errors slip through in pooled workers where I didn't notice rare failures and the loss of a single worker thread. A screaming halt would have woken me up from my lazy stupidity... substantially < $0.02 matt.

On Fri, 26 Aug 2005 09:17:41 -0400, "David Abrahams" <dave@boost-consulting.com> said:

Another possibility (a good one IMO) would be to allow a user-replaceable handler there, which could abort, log a message and then abort, eat the exception, etc., as desired. If the default is to eat the exception, you have perfect backward compatibility.

I don't think any user-replaceable handler strategy, other than through set_terminate() or set_unexpected(), is going to work well. That's because you will never have an option of seeing a function name in the call stack trace of the core file where exception originated. Am I missing something? Slawomir

On Fri, 26 Aug 2005 15:51:44 -0400, "David Abrahams" <dave@boost-consulting.com> said:

A user-replaceable handler can't dump core?

Not with the complete stack trace of where the exception originated. Slawomir

David Abrahams wrote:

"Slawomir Lisznianski" <slisznianski@asyncnet.com> writes:

...

On Fri, 26 Aug 2005 15:51:44 -0400, "David Abrahams" <dave@boost-consulting.com> said:

...

A user-replaceable handler can't dump core?

Not with the complete stack trace of where the exception originated.

Neither can any other mechanism, portably across all implementations, even if you only restrict it to conforming ones. Does unexpected do the right thing on a wide range of compilers? It won't work on MSVC, for example.

Why are we having this discussion at all? :-) I thought that it was "common knowledge" that aborting the process when an exception escapes the thread procedure is better than eating the exception and silently killing the thread. The Sun folks were convinced of that, having tried the catch(...) approach first.

On 26/08/05, David Abrahams <dave@boost-consulting.com> wrote: Matt Hurd <matt.hurd@gmail.com> writes:

...

...

On 25/08/05, John Maddock <john@johnmaddock.co.uk> wrote:

<snip>

Another possibility (a good one IMO) would be to allow a user-replaceable handler there, which could abort, log a message and then abort, eat the exception, etc., as desired. If the default is to eat the exception, you have perfect backward compatibility.

Yes, a user replaceable handler seems best. Perhaps through a policy so a no-op could be chosen but I'm not sure if the small additional complexity of a policy is worth it just to allow the handler to be elided. I'd still prefer that backward compatibility was broken and that the default was catastrophic failure as per a normal exception. Apart from disliking the default of silent failure, I'm not sure main's thread should be special. Main doesn't fail silently. Maybe such a change needs a deprecation period with an notice that the behaviour will change. The majority seem opposed to changing the default though, I seem to be on my own. matt.

Michael Glassford wrote:

Slawomir Lisznianski wrote:

...

Hello,

I find the catch(...) clause in the thread_proxy function (thread.cpp) causing more harm than good.

Does anyone have any comments on this? Support or objections?

A good solution is to provide a customization point: call a handler that can be replaced by the user. You then, from a library point of view, have a well-defined behaviour in presence of uncaught exceptions. Then it's possible for the user to "throw;" in order to further propogate the exception (something I would consider evil).

Mike

Ariel.

Slawomir Lisznianski wrote:

On Thu, 25 Aug 2005 02:12:12 +0200, "Ariel Badichi"

...

A good solution is to provide a customization point: call a handler that can be replaced by the user.

The language already provides this customization through set_unexpected and set_terminate.

But terminate/unexpected functions have special semantics that don't quite fit here. For example, the handler should be able to return.

Althought no standard mandates it, it is commonly expected that each thread can set its own terminate() or unexpected() function -- calling set_terminate() or set_unexpected() in one thread affects only the exceptions in that thread.

Cheers,

Slawomir

Ariel.

On Fri, 26 Aug 2005 09:18:25 -0400, "David Abrahams" <dave@boost-consulting.com> said:

What's expected doesn't matter. It's what's implemented that counts.

I think, in the context of this discussion, it matters quite a lot. What we are debating specifically, is whether a library can allow for customization, yet leave an option of having a default (usually commonly expected) behavior of coring with a full stack trace. The alternative to that, as you pointed out, is to log a message and then abort, or eat the exception, which can easily be done by the user, by her wrapping the `thread' function with the catch-all one. To preserve the compatibility, the thread_proxy could install an exception handler which doesn't call abort(). -- Slawomir Lisznianski

David Abrahams wrote:

My point was that it doesn't matter if people expect that capability when the compiler and/or OS don't provide it.

Your point was obviously valid, but how was stating it getting us any closer to solving the problem at hand (of silent thread death)? What I proposed was for the library to loosen up and let the default, platform specific behavior, kick in. The accompanying Boost.Threads manual could state "if an exception escapes the thread function, the outcome is platform specific". If I'm a user who is not satisfied with the "platform specific behavior", or simply don't know what that behavior is, I will wrap my thread function and make it act one way or another. The important point being: the user will have a choice between a compiler supplied handler and her own.

Arguments about what is "usually commonly expected" are almost always a reflection of the expectation of the person making the argument, and not based on any kind of hard data.

Leveraging compiler features in under- or non-standardized areas seems like a fair objective to me. Slawomir

Michael Glassford schrieb:

Slawomir Lisznianski wrote:

...

Hello,

I find the catch(...) clause in the thread_proxy function (thread.cpp) causing more harm than good.

Does anyone have any comments on this? Support or objections?

I am somewhat surprised by the discussion. 1) How could a user function ever "wrap the code" at the thread_proxy level? There is nothing below on the stack?! Also see: m_thread = reinterpret_cast<void*>(_beginthreadex(0, 0, &thread_proxy, ¶m, 0, &m_id)); The only thing the catch all clause is doing is call the (somewhat superfluous) on_thread_exit() function on the windows platform, which currently only is used for TSS cleanup. Not sure about MPTASKS.) Removing the catch all at this point wouldn't change anything. In particular this would not stop unescaped throws from silently disappearing. An ending thread also does not normally stop the entire process doesn't it? I think the catch all is an artifact from the time before the below DLL_THREAD_DETACH was discovered. On windows the on_thread_exit is called by the operating system in either case: case DLL_THREAD_DETACH: { on_thread_exit(); break; } 2) To stop unescaped exceptions to slip through, one simple should wrap the user thread function with whatever clause is appropriate. And again I don't think it is too bad to forget the re-throw after a catch all at this stage. The on_thread_exit will simply be called later on. But wait, to be honest, there might be a difference: Depending on what the destructors of tls variables are doing, the C runtime library might be left with private leaking tls variables. Supported by the last observation the catch all at the thread proxy level simply is kind of a safeguard to omit this case. As I see it a user function always could catch all and not rethrow, thereby short-circuiting the call to on_thread_exit. So my recommendation is to leave things as they are, since they 1) wouldn't change much and 2) give an additional layer of tls leakage protection. Please correct me if I am on the wrong track. Regards, Roland

(Sorry for quoting so much of the original message, but I felt all of this context was necessary for the fairest and most comprehensible reply.) Roland Schwarz wrote: <snip>

After having read: http://sourceforge.net/tracker/index.php?func=detail&aid=1274707&group_id=7586&atid=357586

And the summary there:

...

In summary, I believe the current behavior is: 1. Dangerous - hides program errors in a most un-exception-like manner.

I don't think so. Please explain why errors could be hidden.

You don't see why I consider silently swallowing unhandled exceptions to be effectively hiding program errors? I'll try my best to support that claim, but I think we may have encountered a fundamental difference of opinions.

...

2. Unfriendly - defeats useful debugging functionality, on some platforms.

I understand this, and agree.

...

3. Surprising - users don't expect libraries to inhibit propagation of their exceptions.

I can see this only in the light of 2) since the only target where the exception could be propagated to, is the debugger.

<snip>

Since the catch all only is effective on windows and only guards against memory leakage in a case where the program is in questionable state anyways (uncaught exception), I would restate my recommendation to remove the catch clause entirely.

I think the debuuger argument is much stronger than the safe-guarding against memory leakage in this case.

Actually, I see no case for worrying about resource deallocation, here. To me, it's not much different than installing a handler for SIGSEGV (the signal UNIX processes receive when they try to access memory outside of their address space), to do the same thing. The only reasonable thing to do, besides exit & let the OS reclaim your resources, is try to perform some action (logging, etc.) to aid with postmortem analysis (which users of boost::thread can do, by a number of means, in cases where dropping into an interactive debugger or collecting a core file aren't options). I equate a memory access violation with allowing an exception to propagate outside of a user's threadfunc, because both generally represent a violated assumption. The key difference is that exceptions provide a more flexible mechanism for the addressing the problem at higher levels (that, and the problem that triggered the exception is more likely to be a problem that can be addressed). If the exception is never caught, then you can't assume the underlying problem was ever addressed - you can assume only that the process may be in an unknown, bad state. Given that the problem may not have been addressed, you can't assume the program can continue to operate correctly. Whether the exception was triggered by bad state or simply by receiving bad/unknown/unhandled input, it seems like the right thing to do is to inform the user that the program failed to behave as expected/requested. In other words, simply continue to defer the error to the next higher level - the user (or whatever higher level entity invoked the program - be it a script, another program, an OS facility, etc.). I regard this as a logical extension of normal exception behavior. To approach it from another perspective, ask 100 C++ programmers the question: "What happens when an unhandled exception is thrown?" I'm willing to bet that the vast majority will say "abnormal program termination", without even considering that the thread triggering the throw might be relevant. Given this assumption about exception behavior, if my program exits cleanly, I'm going to assume that no unhandled exceptions were thrown (i.e. no detectable errors occurred which trigger an exception to be thrown that I don't handle). For most of us, this is a useful approximation of correct operation (i.e. though a clean exit doesn't exactly tell me what did happen, it does tell me what didn't happen - any of the various unhandled errors I judged probable enough to bother checking for). Since it deviates from exception behavior in the main program thread, I see this aspect of boost::thread's behavior surprising (meaning it violates fundamental & I think reasonable assumptions programmers make). Since systems behaving in an unanticipated manner is a common source of design & operator error, I believe this also makes it dangerous. As always, I appreciate the opportunity to discuss these issues. I will try to continue to participate, so long as the discussion remains productive (i.e. as long as I feel I can contribute new information and views, on the matter). That said, it seems both sides of the matter have been pretty thoroughly articulated. Matt Gruenke Software Engineer IntelliVid Corp.

"John Maddock" <john@johnmaddock.co.uk> writes:

Another query: couldn't we make whether exceptions get swallowed a runtime parameter? As in:

if(swallow_exceptions) { try{ users_thread_proc(); }catch(...){ whatever(); } } else users_thread_proc();

I realise this doesn't fit into the current API, but we are considering rewriting/improving Boost.Threads right?

Would adding a runtime parameter improve the API? -- Dave Abrahams Boost Consulting www.boost-consulting.com

John Maddock <john@johnmaddock.co.uk> wrote:

Well that's a good question: I'm just thinking out loud really, the options are choice or no choice, and if choice then compile-time or runtime? My gut feeling is that compile time parameterisation should be used only when there's some real benefit, and I don't see it in this case - there's no need to make everything a template :-) No

I would argue that there is *big* difference in semantics of thread execution between silently swallowing exceptions and leaking them B.

"John Maddock" <john@johnmaddock.co.uk> writes:

...

Would adding a runtime parameter improve the API?

Well that's a good question: I'm just thinking out loud really, the options are choice or no choice, and if choice then compile-time or runtime? My gut feeling is that compile time parameterisation should be used only when there's some real benefit, and I don't see it in this case - there's no need to make everything a template :-) No honestly there isn't really :->

Yeah, but as has been pointed out repeatedly, the user can add his own handler.

The reason I threw this one in, is that there was a suggestion (from Kevlin Henney's thread proposal if I remember correctly) to decouple thread object creation, from the actual execution of the thread, so that then we can tweak the threads properties with getters/setters before we execute the thread (think thread priority, scheduling policy etc).

Does this make sense or am I barking up the wrong tree?

Usually when I hear "let's make an instance in a special not-fully-functional state so we can tweak it with getters and setters before we really bring it to life," little alarm bells go off. It complicates the object invariant and injects often-unwanted state into the program. How about using the parameter library to set these things up? -- Dave Abrahams Boost Consulting www.boost-consulting.com

...

Well that's a good question: I'm just thinking out loud really, the options are choice or no choice, and if choice then compile-time or runtime? My gut feeling is that compile time parameterisation should be used only when there's some real benefit, and I don't see it in this case - there's no need to make everything a template :-) No honestly there isn't really :->

Yeah, but as has been pointed out repeatedly, the user can add his own handler.

Agreed, I think it's time to give up and accept the inevitable here :-)

...

The reason I threw this one in, is that there was a suggestion (from Kevlin Henney's thread proposal if I remember correctly) to decouple thread object creation, from the actual execution of the thread, so that then we can tweak the threads properties with getters/setters before we execute the thread (think thread priority, scheduling policy etc).

Does this make sense or am I barking up the wrong tree?

Usually when I hear "let's make an instance in a special not-fully-functional state so we can tweak it with getters and setters before we really bring it to life," little alarm bells go off. It complicates the object invariant and injects often-unwanted state into the program. How about using the parameter library to set these things up?

Good point, that would be a good match for this kind of task, John.

John Maddock schrieb:

One thing I'm still very clear on: if this change is made we need a big bold statement to that effect in the docs, it's bound to bite somebody, particularly those folks who run automated tests.

I do not believe that removing the entire try catch all will be noticeable from outside! But perhaps I am caught by a fundamental misconception about exceptions and threads. The only thing it changes is behaviour from platform dependant to library defined. Which for the most part will result in the same. To be honest, I did not try out yet what really happens on the various platforms when an exception escapes from a thread. However I rather would suggest somethin along the lines: try { // init platform thread thereadfunc(); } catch (...) { _uncaught_handler(); } But perhaps this is already the standard behaviour, altough I hardly can believe that this has anything to do with ISO, as threading is not currently covered at all. Regards, Roland

John Maddock schrieb:

Another query: couldn't we make whether exceptions get swallowed a runtime parameter? As in:

if(swallow_exceptions) { try{ users_thread_proc(); }catch(...){ whatever(); } } else users_thread_proc();

Unfortunately I was posting my first response to the wrong place. Wouldn't it be a viable alternative to leave out the swallowed exceptions just in debug build? At least the original posters intent (as I understand it) was to get useful information propagated to the debugger. As a pragmatic approach we just could try to run a regression cycle with the catch-all removed and measure how many modules are failing because of this. If the outcome shows no big difference the catch-all could be simply removed. What do you think? Is this feasible? Regards, Roland

Wouldn't it be a viable alternative to leave out the swallowed exceptions just in debug build? At least the original posters intent (as I understand it) was to get useful information propagated to the debugger. As a pragmatic approach we just could try to run a regression cycle with the catch-all removed and measure how many modules are failing because of this. If the outcome shows no big difference the catch-all could be simply removed. What do you think? Is this feasible?

Personally I'm very uncomfortable in changing the semantics of the lib based on whether it's a debug build or not. John.

Matt Gruenke schrieb:

You don't see why I consider silently swallowing unhandled exceptions to be effectively hiding program errors? I'll try my best to support that claim, but I think we may have encountered a fundamental difference of opinions.

Of course I do see a problem in "swallowing unhandled exceptions to be effectively hiding program errors". I don't think there is an issue of opinions. What I am trying to say is simply: Wheter the catch all is present or not will _not_ help to address this problem! This is a technical issue only. What woul help is calling some (possibly user supplied) handler from the catch clause. (Or leave it to the debugger to do something reasonably.) But perhaps I did not understand correctly your point and we both effectively are suggesting the very same? About the leaking issue: I have spent quite some time to help in the implementation of the tls part on the windows platform (to be precise: the addition of the static linkability of the thread lib.) From then I know that there might be a minor issue with this leak. I did not ever claim that this is a serious issue. I just tried to point out that the only usage of the catch clause at the moment is to avoid this. What I see as a technical problem is how you would be able to force a process shutdown from the context of a thread? Could you please try to sketch how this could be done? Could you please restate your suggestion? Remove the catch all or Put a handler function into the catch clause or ....? Regards Roland

David Abrahams schrieb:

Not from a portable C++ point-of-view, no. A platform or C++ implementation is free to implement its own swallowing, so there's no guarantee that there will be less error masking.

I am not sure I got the point, please keep patient: Wouldn't this imply that a particular C++ compiler does know about threading at a specific platform? I always was thinking that current compilers are thread unaware at all? Only libraries e.g. built around pthreads made the connection between C++ and threading so far. Could you please give me an example where this is substantially different? I.e. where the ending of a thread will not swallow uncaught exceptions? But please don't get me wrong. I am not advocating to remove the catch all. I can see the potentially undefined behaviour if it were removed. I just don't think that it will have a big impact from a practical perspective. Also I cannot understand how a C++ compiler should be able to detect that a thread has ended and call the respective process shutdown if there are any uncaught exceptions. This also cannot be delegated to the operating system, since this would imply that any ending thread will tear down the process, which obviously cannot be true. Operating systems are not exception aware, are they? (Windows is an exception here, but the exception mechanism is different from C++ exceptions.) Consequently uncaught exceptions will silently disapear (swallow?).

...

What I see as a technical problem is how you would be able to force a process shutdown from the context of a thread? Could you please try to sketch how this could be done?

You can just put the try/catch in your thread main function and call the shutdown function from there.

But this would force a user to use platform specific code in his program, wouldn't it? The boost thread does not have a dedicated function for process tear down. And I doubt the standard does say anything about process tear down in the presence of threads. On the other hand the process tear down could be put into the catch all clause of the thread proxy. Unfortunately this will be a _big_ change from a user perspective. What about adding a dedicated process tear down function to the library? This function could be called by the user if needed. Regards Roland

David Abrahams writes:

Roland Schwarz writes:

...

But please don't get me wrong. I am not advocating to remove the catch all.

I am.

As I already posted in a previous mail: I also do not have objections against removing it. We both seemingly changed our minds to follow the arguments of the original poster. But you think it will have a big impact, and I do not know for sure. I would have suggested to roll out a small test for the currently supported compilers, to see how big the impact would be to regressions. But as you say: "With most of the compilers I use an uncaught exception thrown from a user thread will terminate the application", we can skip this step and assume it for given.

...

Also I cannot understand how a C++ compiler should be able to detect that a thread has ended and call the respective process shutdown if there are any uncaught exceptions.

It's known as "compiler magic."

Hmm, hmm, I am definitely not educated enough in this discipline ;-)

Urr, abort(), terminate(), and exit() all come about as close as you could want to addressing process termination.

Yes, this is most likely the case. And if you mean that this should happen from the users program, I am with you. On the other hand the standard has nothing to say what this will cause when called from a thread. No wait: Calling the function void exit(int); terminates the program without leaving the current block <...> If exit is called to end a program during the destruction of an object with static storage duration, the program has undefined behaviour. So when calling this function (e.g. as a result to an exception) during the main thread is ending the program and inside a global destructor the program has undefined behaviour. I don't know if this is any different than the current situation.

And having any catch all clause at all will prevent us from reaching other goals desired by the instigators of this thread.

Another possible approach would be to do differently in a debug build, since the only goal the original poster seems to aim is to get meaningful debug information. I am tempted to compare this to stack probes, no mans land guards for malloc (sorry new) and the like, which are also normally only used during debug builds. These features all have the potential to cause error slip through in release builds. But of course it would be good to know that regressions also run with the catch clause removed. Or do you expect much fails? This would be quite contrary to the claim that 100% of the C++ programmers are expecting an uncaught thread exception to tear down the process. Do you think it would be possible to just try this out? I mean to schedule an extra regression cycle with the catch all removed and see how much of the code is affected?

...

What about adding a dedicated process tear down function to the library? This function could be called by the user if needed.

What's wrong with what the standard provides?

Nothing besides not addressing threading. Regards, Roland

David Abrahams schrieb:

Roland Schwarz <roland.schwarz@chello.at> writes:

...

and I do not know for sure. I would have suggested to roll out a small test for the currently supported compilers, to see how big the impact would be to regressions. But as you say: "With most of the compilers I use an uncaught exception thrown from a user thread will terminate the application",

I didn't ever say that.

I have to apologize. I mixed up the posters. The sentence is attributed to John Maddok:

I think I'd come down on the other side: this isn't a minor change, it's a complete change of semantics and we should be very careful about such a change. With most of the compilers I use an uncaught exception thrown from a user thread will terminate the application, which may be what you want, or it may not: but it sure is drastic! You can always trap uncaught exceptions yourself and call abort (or whatever) if that's what you want. The debugger I use will also let you trap all thrown exceptions if that's what you want, whether they all do that I don't know.

I hope you didn't see this as beeing offensive. Regards, Roland