Bug 28145 - C++ (throw() and catch(...) {/* fall through */ } ) and pthread cancellation are incompatible (at least with NPTL)
Summary: C++ (throw() and catch(...) {/* fall through */ } ) and pthread cancellation...
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: other (show other bugs)
Version: 4.2.0
: P3 minor
Target Milestone: ---
Assignee: Jason Merrill
URL:
Keywords:
Depends on:
Blocks: 53377
  Show dependency treegraph
 
Reported: 2006-06-23 18:00 UTC by Daniel Jacobowitz
Modified: 2012-06-22 08:44 UTC (History)
12 users (show)

See Also:
Host:
Target: *-*-linux*
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments
libstdc++ patch to allow catching forced unwind separately (1.39 KB, patch)
2006-07-31 09:38 UTC, Jason Merrill
Details | Diff
NPTL patch to allow discarding cancellation exception (950 bytes, patch)
2006-07-31 09:40 UTC, Jason Merrill
Details | Diff
libstdc++ patch to prevent ... from catching forced unwird (463 bytes, patch)
2006-08-01 02:13 UTC, Jason Merrill
Details | Diff
revision to forced-lib.patch that also adds __foreign_exception (1.46 KB, patch)
2006-08-01 02:33 UTC, Jason Merrill
Details | Diff
NPTL patch for sticky cancellation (1.48 KB, patch)
2006-08-01 21:08 UTC, Jason Merrill
Details | Diff
final __forced_unwind patch and fixes applied in 2007, for historical reference (6.16 KB, patch)
2010-07-16 19:21 UTC, Jason Merrill
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Jacobowitz 2006-06-23 18:00:41 UTC
(This is not a new problem, and everyone involved is familiar with it.  I was just surprised not to find a record of it in bugzilla.)

On targets using a recent version of glibc and the NPTL threading package, if you cancel a thread using pthread_cancel while it is writing to an ostream, you'll get a fatal error and an abort from glibc.  The abort happens because std::ostream::put uses catch(...), catches the cancellation exception, and does not rethrow it.  You can write a nasty workaround for this, but I'm obviously not going to recommend it:

int cancelled = 0;                                                                                    
try {                                                                                                 
 pthread_cleanup_push(something-to-set-cancelled);                                                   
  write()                                                                                             
  pthread_cleanup_pop(discard-cleanups);                                                              
} catch (...) {                                                                                       
  if (cancelled) throw;                                                                               
  normal-catch-handling;
}                                                                                                     

That would only work if there was nothing marked throw() on the way up, of course.

This problem has been discussed at length on the c++-pthreads list, but I don't think a consensus was really reached.
Comment 1 Mark Mitchell 2006-06-23 18:14:50 UTC
Subject: Re:   New: libstdc++ and pthread cancellation
 are incompatible (at least with NPTL)

drow at gcc dot gnu dot org wrote:

> On targets using a recent version of glibc and the NPTL threading package, if
> you cancel a thread using pthread_cancel while it is writing to an ostream,
> you'll get a fatal error and an abort from glibc.

> This problem has been discussed at length on the c++-pthreads list, but I don't
> think a consensus was really reached.

I believe that to solve this particular problem, a possible compromise
might be:

(1) Give the thread-cancellation exception a well-known name.
(2) Permit catching that exception -- but only if it is rethrown.

The point of (2) is that it avoids the controversy about whether or not
you are permitted to catch, and then not rethrow, the exception.

Then, you could write:

  try {
    io ();
  } catch (__thread_cancel&) {
    throw;
  } catch (...) {
    /* Normal catch handling.  */
  }

That would provide a mechanism for making libstdc++ (and other
applications) cancel-safe.  It would still not solve the general
philosophical issues about whether or not you should be allowed to fail
to "eat" the cancellation exception, but I think it would be a step forward.

Another way to do this, instead of having a named exception, would be to
have a "__uncaught_cancellation()" function, similar to
"std::__uncaught_exception()".  Then, you would do:

  try {
    io ();
  } catch (...) }
    if (__uncaught_cancellation ())
      throw;
    /* Normal catch handling.  */
  }

Comment 2 Andrew Pinski 2006-06-24 01:43:41 UTC
Actually I think the consensus was talk to the C++ and POSIX standards comittee about this case. 

 It is not just libstdc++ that could cause problems but any program that uses throw() or catch(...) {/* fall through */ }.

In fact I can come up with a simple C++ case which fails without using libstdc++.
Comment 3 Daniel Jacobowitz 2006-06-24 02:52:56 UTC
No, that was not the consensus, and I reported this bug specifically because the coding practices used in libstdc++ cause problems with cancellation... progress was being made, the last time I read c++-pthreads, but trailed off.
Comment 4 Mark Mitchell 2006-06-24 04:10:23 UTC
Subject: Re:  C++ (throw() and catch(...) {/*  fall through
 */ } ) and pthread cancellation are incompatible (at least with NPTL)

drow at gcc dot gnu dot org wrote:
> ------- Comment #3 from drow at gcc dot gnu dot org  2006-06-24 02:52 -------
> No, that was not the consensus, and I reported this bug specifically because
> the coding practices used in libstdc++ cause problems with cancellation...
> progress was being made, the last time I read c++-pthreads, but trailed off.

I don't think it's necessary (or even useful) to wait for the standards
bodies to solve this problem.  In practice, the c++-pthreads discussion
died off, and the C++ POSIX reflector has had no activity since January.
 So, it's certainly going to be a while before we have standards
documents to rely upon.

However, we know that it should be possible to write cancel-safe C++
libraries (including, in particular, libstdc++); otherwise, it's hard to
use C++ in a multi-threaded application.  And, we know that we can do it
with a very simple hook: we just need a way to ask whether the current
thread is being cancelled.  GLIBC has lots of extensions; if we can have
__thread_cancelled(), we've got enough, without having to solve the
problem of whether or not it should be possible to catch (and not
rethrow) the thread-cancellation exception.  (I'd rather have a name for
the thread exception, as that seems more natural, but this is a
difference only of syntax; the hook function would be a perfectly
satisfactory way to make progress.)

Comment 5 Paolo Carlini 2006-07-07 17:17:52 UTC
(In reply to comment #4)
> However, we know that it should be possible to write cancel-safe C++
> libraries (including, in particular, libstdc++); otherwise, it's hard to
> use C++ in a multi-threaded application.  And, we know that we can do it
> with a very simple hook: we just need a way to ask whether the current
> thread is being cancelled.  GLIBC has lots of extensions; if we can have
> __thread_cancelled(), we've got enough, without having to solve the
> problem of whether or not it should be possible to catch (and not
> rethrow) the thread-cancellation exception.  (I'd rather have a name for
> the thread exception, as that seems more natural, but this is a
> difference only of syntax; the hook function would be a perfectly
> satisfactory way to make progress.)

FWIW my personal opinion about this issue, I concur. Now, assuming all the knowledgeable people do indeed agree, is it really feasible to add such feature to glibc? Is someone going to ask glibc maintainers opinion?
Comment 6 Jason Merrill 2006-07-31 09:38:13 UTC
Created attachment 11978 [details]
libstdc++ patch to allow catching forced unwind separately

Trying to move this issue forward: here's a libsupc++ patch that allows users to detect forced unwind using a magic class.  This is a little ugly, as there isn't actually an object for the catch parameter to bind to.  It should also be discussed with the C++ ABI committee; I notice that the ABI EH document still says  that you can't catch forced unwind (like Ada).
Comment 7 Jason Merrill 2006-07-31 09:40:23 UTC
Created attachment 11979 [details]
NPTL patch to allow discarding cancellation exception

And here's a completely untested patch to NPTL to implement the "sticky cancel" semantics I've talked about; discarding the forced unwind exception causes the exiting bit to be unset, so that the next cancellation point starts unwinding again.
Comment 8 Jason Merrill 2006-08-01 02:13:06 UTC
Created attachment 11985 [details]
libstdc++ patch to prevent ... from catching forced unwird

Finally, this patch stops ... from catching forced unwind, as specified by the ABI.

I think the catch (abi::__forced_unwind &) patch should be uncontroversial, and works well with either solution to the catch (...) problem.
Comment 9 Jason Merrill 2006-08-01 02:33:26 UTC
Created attachment 11986 [details]
revision to forced-lib.patch that also adds __foreign_exception
Comment 10 Daniel Jacobowitz 2006-08-01 02:47:55 UTC
Subject: Re:  C++ (throw() and catch(...) {/*  fall through */ } ) and pthread cancellation are incompatible (at least with NPTL)

On Tue, Aug 01, 2006 at 02:13:08AM -0000, jason at gcc dot gnu dot org wrote:
> Finally, this patch stops ... from catching forced unwind, as specified by the
> ABI.

Just making sure I understand - catch (...) { foo(); throw; } will no
longer call foo during forced unwinding, only destructors and explicit
forced unwinding catches will be called?

[What does this imply for throw()?]

Comment 11 Jason Merrill 2006-08-01 07:10:49 UTC
Subject: Re:  C++ (throw() and catch(...) {/*  fall through
 */ } ) and pthread cancellation are incompatible (at least with NPTL)

drow at gcc dot gnu dot org wrote:
> Just making sure I understand - catch (...) { foo(); throw; } will no
> longer call foo during forced unwinding, only destructors and explicit
> forced unwinding catches will be called?

Yes.  I should clarify that I'm just implementing all the different 
options, in hopes that helps to move the discussion along.

> [What does this imply for throw()?]

throw() will always block unwinding.  For that case "sticky" 
cancellation works better.  But I don't think that's actually an issue 
for iostreams, is it?

Jason
Comment 12 Daniel Jacobowitz 2006-08-01 13:02:37 UTC
Subject: Re:  C++ (throw() and catch(...) {/*  fall through */ } ) and pthread cancellation are incompatible (at least with NPTL)

On Tue, Aug 01, 2006 at 07:10:53AM -0000, jason at redhat dot com wrote:
> > [What does this imply for throw()?]
> 
> throw() will always block unwinding.  For that case "sticky" 
> cancellation works better.  But I don't think that's actually an issue 
> for iostreams, is it?

Right, I don't think so.

Comment 13 Jason Merrill 2006-08-01 21:08:09 UTC
Created attachment 11988 [details]
NPTL patch for sticky cancellation

Revised, tested patch with testcase.

So, there are 3 patches here, which are fairly independent of each other:

1) catch (abi::__forced_unwind &) -- this patch seems uncontroversial and I think should go in very soon.
2) prevent catch (...) from catching forced unwind -- this change is highly controversial.  The last time I brought up this topic I tried to convince C++ folks to subscribe to this perspective and failed.
3) "sticky cancel" (if cancellation exception is discarded, raise a new one at the next cancellation point) -- Ulrich was strongly opposed to discarding the cancellation exception in the past, but might be more open now that I have an implementation.

As previously mentioned, #3 is the only way to deal with cancellation within throw().  Cancellation will ignore other exception specifications.

#2 or #1 are enough to deal with the catch (...) problem.  #3 alone is not; it can lead to an infinite loop of catch and discard in an outer loop that tries to recover from exceptions.

#1 seems like a good thing regardless of #2 or #3.
Comment 14 Howard Hinnant 2006-11-01 23:33:19 UTC
So swallowing a cancel-exception (in C++) is sometimes the right thing to do.

Imagine a thread pool executing a queue of tasks.  These tasks can well have handles so that clients can wait/join with results in the future.  Such results could be normal or exceptional.  If you've got a hung task in a thread pool, maybe you want to cancel it.  That means the task should end, but once the OS thread dumps the task, records the exception that ended it (cancellation), the thread should be free to get the next task out of the queue and execute it.
Comment 15 Jason Merrill 2007-04-13 20:13:19 UTC
Subject: Re:  C++ (throw() and catch(...) {/*  fall through
 */ } ) and pthread cancellation are incompatible (at least with NPTL)

Howard's example seems to me like an argument for not necessarily using 
thread cancellation to abort a task, but not for discarding the cancel 
request entirely.  A cancellation request is asking for a thread to go 
away; if you want to send a different message use a different mechanism.

Jason
Comment 16 Howard Hinnant 2007-04-15 16:13:51 UTC
(In reply to comment #15)

This makes clean up / rethrow during cancellation awkward.  Code would have to check for two (or more) different kinds of cancellation:  Am I executing in an OS thread, or in a thread pool?
Comment 17 Jason Merrill 2007-04-15 19:01:00 UTC
Subject: Re:  C++ (throw() and catch(...) {/*  fall through
 */ } ) and pthread cancellation are incompatible (at least with NPTL)

hhinnant at apple dot com wrote:
> This makes clean up / rethrow during cancellation awkward.  Code would have to
> check for two (or more) different kinds of cancellation:  Am I executing in an
> OS thread, or in a thread pool?

Well, yes.  The mechanism for pthread_cancel won't necessarily work with 
other forms of cancellation.

Doing clean up in a catch(...) block has always been inelegant, a 
destructor will be more reliable.  I don't understand why we aren't 
adding finally in this round of standardization...

Comment 18 Jason Merrill 2007-05-07 22:27:59 UTC
Subject: Bug 28145

Author: jason
Date: Mon May  7 21:27:54 2007
New Revision: 124517

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=124517
Log:
        PR c++/28145
        * libsupc++/cxxabi.h (__forced_unwind, __foreign_exception):
        New classes.
        * libsupc++/eh_exception.cc: Define their destructors.
        * config/abi/pre/gnu.ver: Export their type_infos.
        * config/abi/pre/gnu-versioned-namespace.ver: Likewise.
        * libsupc++/eh_personality.cc: A handler for abi::__forced_unwind
        matches a forced unwind, and a handler for abi::__foreign_exception
        matches a foreign exception.

        * include/bits/istream.tcc: Rethrow forced unwind.
        * include/bits/ostream.tcc: Likewise.
        * include/bits/ostream_insert.h: Likewise.
        * include/bits/basic_string.tcc (operator>>, getline): Likewise.
        * include/bits/fstream.tcc (basic_filebuf::close): Likewise.
        * include/ext/vstring.cc (operator>>, getline): Likewise.
        * src/istream.cc: Likewise.
        * src/compatibility.cc (basic_istream::ignore): Likewise.
        * include/std/bitset (operator>>): Likewise.
        * include/std/fstream (basic_filebuf::close): Remove throw() spec.
        * libsupc++/cxxabi-internal.h: Split out from...
        * libsupc++/cxxabi.h: ...here.

Added:
    trunk/gcc/testsuite/g++.dg/abi/forced.C
    trunk/libstdc++-v3/libsupc++/cxxabi-internal.h
      - copied, changed from r124509, trunk/libstdc++-v3/libsupc++/cxxabi.h
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/config/abi/pre/gnu-versioned-namespace.ver
    trunk/libstdc++-v3/config/abi/pre/gnu.ver
    trunk/libstdc++-v3/config/locale/gnu/monetary_members.cc
    trunk/libstdc++-v3/include/bits/basic_string.tcc
    trunk/libstdc++-v3/include/bits/fstream.tcc
    trunk/libstdc++-v3/include/bits/istream.tcc
    trunk/libstdc++-v3/include/bits/locale_classes.h
    trunk/libstdc++-v3/include/bits/ostream.tcc
    trunk/libstdc++-v3/include/bits/ostream_insert.h
    trunk/libstdc++-v3/include/debug/deque
    trunk/libstdc++-v3/include/ext/pb_ds/detail/resize_policy/hash_load_check_resize_trigger_imp.hpp
    trunk/libstdc++-v3/include/ext/vstring.tcc
    trunk/libstdc++-v3/include/std/bitset
    trunk/libstdc++-v3/include/std/fstream
    trunk/libstdc++-v3/include/tr1/hypergeometric.tcc
    trunk/libstdc++-v3/libsupc++/cxxabi.h
    trunk/libstdc++-v3/libsupc++/eh_exception.cc
    trunk/libstdc++-v3/libsupc++/eh_personality.cc
    trunk/libstdc++-v3/src/compatibility.cc
    trunk/libstdc++-v3/src/ios.cc
    trunk/libstdc++-v3/src/ios_init.cc
    trunk/libstdc++-v3/src/istream.cc

Comment 19 Jason Merrill 2010-07-16 19:21:38 UTC
Created attachment 21227 [details]
final __forced_unwind patch and fixes applied in 2007, for historical reference
Comment 20 nightstrike 2012-06-22 02:44:36 UTC
Give this:
http://thread.gmane.org/gmane.comp.gcc.help/41456

Will further work happen on this PR, Jason?