[Bug libstdc++/45133] New: [c++0x] std::future will crash with NULL deref if get() is called twice

[Bug libstdc++/45133] New: [c++0x] std::future will crash with NULL deref if get() is called twice

[lloyd at randombit dot net]

The following quick snippet crashes with GCC 4.5.0, on the second call to
get():

"""
#include <future>

int make_int() { return 52; }

int main()
{
  std::future<int> future_in = std::async(make_int);

  printf("%d\n", future_in.get());
  printf("%d\n", future_in.get());
}
"""

Backtrace:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000401c3f in std::__future_base::_State::wait (this=0x0) at
/usr/lib/gcc/x86_64-pc-linux-gnu/4.5.0/include/g++-v4/future:281
281             _M_run_deferred();
(gdb) backtrace 
#0  0x0000000000401c3f in std::__future_base::_State::wait (this=0x0) at
/usr/lib/gcc/x86_64-pc-linux-gnu/4.5.0/include/g++-v4/future:281
#1  0x0000000000402e9d in std::__basic_future<int>::_M_get_result
(this=0x7fffffffd880)
    at /usr/lib/gcc/x86_64-pc-linux-gnu/4.5.0/include/g++-v4/future:515
#2  0x0000000000402560 in std::future<int>::get (this=0x7fffffffd880) at
/usr/lib/gcc/x86_64-pc-linux-gnu/4.5.0/include/g++-v4/future:594
#3  0x00000000004017a2 in main () at promise.cpp:10

Obviously this is not a valid operation, since get() moves the result and then
zaps itself (and valid() returns false after get() is called). However from a
usability standpoint this doesn't seem ideal. It looks like adding a check in
__basic_future::_M_get_result that ensures that _M_state is not NULL would be
sufficient to catch this case.

I would assume the result of doing a get() when !valid() is undefined, so
throwing an exception when someone does this would be conforming, and a lot
more obvious and friendly. If for some reason this couldn't work, even just an
assertion fail would be more informative than a NULL pointer deref.


-- 
           Summary: [c++0x] std::future will crash with NULL deref if get()
                    is called twice
           Product: gcc
           Version: 4.5.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: lloyd at randombit dot net
 GCC build triplet: x86_64-pc-linux-gnu
  GCC host triplet: x86_64-pc-linux-gnu
GCC target triplet: x86_64-pc-linux-gnu


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45133

[Bug libstdc++/45133] [c++0x] std::future will crash with NULL deref if get() is called twice

[paolo dot carlini at oracle dot com]

------- Comment #1 from paolo dot carlini at oracle dot com  2010-07-29 17:34 -------
Jon, can you have a look? Thanks.


-- 

paolo dot carlini at oracle dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jwakely dot gcc at gmail dot
                   |                            |com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45133

[Bug libstdc++/45133] [c++0x] std::future will crash with NULL deref if get() is called twice

[redi at gcc dot gnu dot org]

------- Comment #2 from redi at gcc dot gnu dot org  2010-07-30 14:30 -------
(In reply to comment #0)
> 
> I would assume the result of doing a get() when !valid() is undefined,

No need to assume, it's stated explicitly in the FCD.

> so
> throwing an exception when someone does this would be conforming, and a lot
> more obvious and friendly. If for some reason this couldn't work, even just an
> assertion fail would be more informative than a NULL pointer deref.

Confirming, as an enhancement request.

I'm travelling until Tuesday so won't look in detail right now, but IIRC there
are possible race conditions so it's not necessarily as simple as just checking
for a NULL pointer.


-- 

redi at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |redi at gcc dot gnu dot org
                   |dot org                     |
           Severity|normal                      |enhancement
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2010-07-30 14:30:35
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45133

[Bug libstdc++/45133] [c++0x] std::future will crash with NULL deref if get() is called twice

[redi at gcc dot gnu dot org]

------- Comment #3 from redi at gcc dot gnu dot org  2010-07-30 14:39 -------
On second thoughts, concurrent calls to future::get are also undefined, so
simply asserting valid() would be better.  I'll do that asap.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45133

[Bug libstdc++/45133] [c++0x] std::future will crash with NULL deref if get() is called twice

[redi at gcc dot gnu dot org]

------- Comment #4 from redi at gcc dot gnu dot org  2010-08-06 15:36 -------
The committee is currently in the middle of re-designing future::get so I'll
wait and see what happens.  It looks as though it's going to be renamed and
throw if called twice.


-- 

redi at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |SUSPENDED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45133

[Bug libstdc++/45133] [c++0x] std::future will crash with NULL deref if get() is called twice

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45133

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.6.0

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-12-14 23:33:17 UTC ---
fixed for 4.6.0 by throwing as per the new note in the latest draft

[Bug libstdc++/45133] [c++0x] std::future will crash with NULL deref if get() is called twice

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45133

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-12-14 23:27:20 UTC ---
Author: redi
Date: Tue Dec 14 23:27:17 2010
New Revision: 167823

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=167823
Log:
2010-12-14  Jonathan Wakely  <jwakely.gcc@gmail.com>

    PR libstdc++/45133
    * include/std/future (__basic_future::wait): Throw if not valid.
    (__basic_future::wait_for): Likewise.
    (__basic_future::wait_until): Likewise.
    (__basic_future::_M_get_result): Likewise.
    * testsuite/30_threads/future/members/45133.cc: New.
    * testsuite/30_threads/shared_future/members/45133.cc: New.

Added:
    trunk/libstdc++-v3/testsuite/30_threads/future/members/45133.cc
    trunk/libstdc++-v3/testsuite/30_threads/shared_future/members/45133.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/std/future

[Bug libstdc++/45133] [c++0x] std::future will crash with NULL deref if get() is called twice

[redi at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45133

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|SUSPENDED                   |ASSIGNED

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> 2010-12-03 15:38:53 UTC ---
The latest draft still says this is undefined behaviour, but has a note
encouraging implementations to detect this case and throw.  I am suitably
encouraged.

I'll start by doing that conditionally when _GLIBCXX_DEBUG is defined...