base::Unretained() isn't forced for non-member raw pointers passed to Bind() |
||||
Issue description
This code compiles (ref. TestIOThread::PostTaskAndWait):
base::WaitableEvent event(WaitableEvent::ResetPolicy::AUTOMATIC,
WaitableEvent::InitialState::NOT_SIGNALED);
task_runner()->PostTask(from_here,
base::Bind(&PostTaskAndWaitHelper, &event, task));
event.Wait();
and it shouldn't because WaitableEvent isn't ref-counted and thus &event should be wrapped in base::Unretained() -- or so the documentation says: https://cs.chromium.org/chromium/src/base/bind_helpers.h?rcl=0&l=55
This used to be enforced at compile time FWIR, what happened to this? How can we add tests for this (i.e. testing failure to compile is hard I assume -- maybe clang has some magic thing)?
@tzik?
,
Jul 18 2016
,
Jul 18 2016
|this| pointers are forced to be wrapped by Unretained() or be a ref-counted object. However, other parameters are not enforced for historical reason.
,
Jul 18 2016
Oh I assumed this was binding a method on &event, but no it's not.
,
Jul 18 2016
(Making people use Unretained for any pointer might be reasonable to do if not a lot of work)
,
Jul 19 2016
I think forcing base::Unretained() annotation for all pointers would be nice, but I wonder if it'd be a lot of extra noise... basically, I'd be worried about base::Unretained becoming the new normal.
,
Jul 19 2016
Interesting, had never realized Unretained() was only intended for the member pointer on member methods. Isn't the refcount behavior the same for the member pointer as for ref-counted arguments though? Why do we feel the member pointer is special?
,
Jul 19 2016
Bind/Callback code derefs and uses the member pointer, but it just passes other pointers and you're responsible for doing the right thing with them. |
||||
►
Sign in to add a comment |
||||
Comment 1 by danakj@chromium.org
, Jul 18 2016