Commit Graph

27 Commits

Author SHA1 Message Date
aea9293fd4 Revert of Fixing potential AsyncInvoker deadlock that occurs for "reentrant" invocations. (patchset #3 id:40001 of https://codereview.webrtc.org/2885143006/ )
Reason for revert:
Causes a new TSan race warning. Will reland after fixing. Note this is the same race as will be fixed by https://codereview.webrtc.org/2876273002/.

Original issue's description:
> Fixing potential AsyncInvoker deadlock that occurs for "reentrant" invocations.
>
> The deadlock occurs if the AsyncInvoker is destroyed on thread A while
> a task on thread B is running, which AsyncInvokes a task back on thread
> A.
>
> This was causing pending_invocations_ to end up negative, because
> an AsyncClosure that's never added to a thread's message queue (due to
> the "destroying_" flag) caused the count to be decremented but not
> incremented.
>
> BUG=None
>
> Review-Url: https://codereview.webrtc.org/2885143006
> Cr-Commit-Position: refs/heads/master@{#18225}
> Committed: ef37ca5fb3

TBR=nisse@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=None

Review-Url: https://codereview.webrtc.org/2904543003
Cr-Commit-Position: refs/heads/master@{#18248}
2017-05-23 19:55:03 +00:00
ef37ca5fb3 Fixing potential AsyncInvoker deadlock that occurs for "reentrant" invocations.
The deadlock occurs if the AsyncInvoker is destroyed on thread A while
a task on thread B is running, which AsyncInvokes a task back on thread
A.

This was causing pending_invocations_ to end up negative, because
an AsyncClosure that's never added to a thread's message queue (due to
the "destroying_" flag) caused the count to be decremented but not
incremented.

BUG=None

Review-Url: https://codereview.webrtc.org/2885143006
Cr-Commit-Position: refs/heads/master@{#18225}
2017-05-22 22:32:51 +00:00
c4a3173db0 Delete unused features of AsyncInvoke.
Also eliminates its dependency on callback.h.

BUG=None

Review-Url: https://codereview.webrtc.org/2871403003
Cr-Commit-Position: refs/heads/master@{#18163}
2017-05-16 12:51:29 +00:00
30cba07bd1 Delete unused class ComThread.
BUG=None

Review-Url: https://codereview.webrtc.org/2835893003
Cr-Commit-Position: refs/heads/master@{#17857}
2017-04-25 07:09:15 +00:00
37f5ecfd8f Replace NULL with nullptr or null in webrtc/base/.
BUG=webrtc:7147

Review-Url: https://codereview.webrtc.org/2718663005
Cr-Commit-Position: refs/heads/master@{#16878}
2017-02-27 22:06:41 +00:00
162cb53e7b Making AsyncInvoker destructor thread-safe.
The documentation for AsyncInvoker states that it owns the lifetime of
calls, and when its destructor is called, all in-flight calls are
cancelled or finish executing. The "cancelled" part is working, but if
a call is in the middle of executing, the destructor does *not* wait.

This is fixed by keeping a count of pending invocations, which is
decremented when a call is either cleared from a message queue or
finishes executing.

BUG=webrtc:3914, webrtc:3911

Review-Url: https://codereview.webrtc.org/2694723004
Cr-Commit-Position: refs/heads/master@{#16811}
2017-02-24 01:10:07 +00:00
5d97a9a05b Adding more detail to MessageQueue::Dispatch logging.
Every message will now be traced with the location from which it was
posted, including function name, file and line number.

This CL also writes a normal LOG message when the dispatch took more
than a certain amount of time (currently 50ms).

This logging should help us identify messages that are taking
longer than expected to be dispatched.

R=pthatcher@webrtc.org, tommi@webrtc.org

Review URL: https://codereview.webrtc.org/2019423006 .

Cr-Commit-Position: refs/heads/master@{#13104}
2016-06-10 21:17:33 +00:00
bfefb03ec1 Replace scoped_ptr with unique_ptr everywhere
But keep #including scoped_ptr.h in .h files, so as not to break
WebRTC users who expect those .h files to give them rtc::scoped_ptr.

BUG=webrtc:5520

Review-Url: https://codereview.webrtc.org/1937693002
Cr-Commit-Position: refs/heads/master@{#12581}
2016-05-01 21:53:55 +00:00
555604a746 Replace scoped_ptr with unique_ptr in webrtc/base/
This propagated into various other places. Also had to #include headers that
were implicitly pulled by "scoped_ptr.h".

BUG=webrtc:5520

Review URL: https://codereview.webrtc.org/1920043002

Cr-Commit-Position: refs/heads/master@{#12501}
2016-04-26 10:13:28 +00:00
25d1f28fa9 Fix race between Thread ctor/dtor and MessageQueueManager registrations.
This CL fixes a race where for Thread objects the parent MessageQueue
constructor registers the object in the MessageQueueManager even though
the Thread is not constructed completely yet. Same happens during
destruction.

BUG=webrtc:1225

Review URL: https://codereview.webrtc.org/1666863002

Cr-Commit-Position: refs/heads/master@{#11497}
2016-02-05 08:25:04 +00:00
5ad935cb56 Remove mutable from rtc::CriticalSection members.
rtc::CriticalSection is now lockable from const methods and no longer
need to remain mutable.

BUG=
R=tommi@webrtc.org

Review URL: https://codereview.webrtc.org/1613643004

Cr-Commit-Position: refs/heads/master@{#11367}
2016-01-25 11:52:53 +00:00
e2976c87f7 Remove DISABLED_ON_ macros.
Macro incorrectly displays DISABLED_ON_ANDROID in test names for
parameterized tests under --gtest_list_tests, causing tests to be
disabled on all platforms since they contain the DISABLED_ prefix rather
than their expanded variants.

This expands the macro variants to inline if they're disabled or not,
and removes building some tests under configurations where they should
fail, instead of building them but disabling them by default.

The change also removes gtest_disable.h as an unused include from many
other files.

BUG=webrtc:5387, webrtc:5400
R=kjellander@webrtc.org, phoglund@webrtc.org
TBR=henrik.lundin@webrtc.org

Review URL: https://codereview.webrtc.org/1547343002 .

Cr-Commit-Position: refs/heads/master@{#11150}
2016-01-04 21:44:16 +00:00
8c38e8b9b9 Clean up PlatformThread.
* Move PlatformThread to rtc::.
* Remove ::CreateThread factory method.
* Make non-scoped_ptr from a lot of invocations.
* Make Start/Stop void.
* Remove rtc::Thread priorities, which were unused and would collide.
* Add ::IsRunning() to PlatformThread.

BUG=
R=tommi@webrtc.org

Review URL: https://codereview.webrtc.org/1476453002 .

Cr-Commit-Position: refs/heads/master@{#10812}
2015-11-26 16:45:57 +00:00
d9b75bef5d Fix a data race in the thread unit tests.
The flag used in thread_unittest.cc:FunctorB is subject to a (mostly
harmless) data race. In a tsan build, reproduce using

  out/Release/rtc_unittests --gtest_filter=AsyncInvokeTest.FireAndForget

There are additional tsan warnings, not all deterministic, when
running all the rtc_unittets: Some data races related to destructors,
and a locking-order-inversion warning. Hence applying this patch does
not make the unit tests tsan-clean.

I should also add that this is my very first cl, so I'm not at all
familiar with the process.

Review URL: https://codereview.webrtc.org/1439613004

Cr-Commit-Position: refs/heads/master@{#10645}
2015-11-16 08:54:10 +00:00
0c4e06b4c6 Use suffixed {uint,int}{8,16,32,64}_t types.
Removes the use of uint8, etc. in favor of uint8_t.

BUG=webrtc:5024
R=henrik.lundin@webrtc.org, henrikg@webrtc.org, perkj@webrtc.org, solenberg@webrtc.org, stefan@webrtc.org, tina.legrand@webrtc.org

Review URL: https://codereview.webrtc.org/1362503003 .

Cr-Commit-Position: refs/heads/master@{#10196}
2015-10-07 10:23:32 +00:00
a1f590f3b6 Add helper class GuardedAsyncInvoker to protect against thread dying
BUG=webrtc:4909
R=tommi@webrtc.org

Review URL: https://codereview.webrtc.org/1303443003 .

Cr-Commit-Position: refs/heads/master@{#9745}
2015-08-20 14:42:50 +00:00
e93cbd13d5 Fix data races in ThreadTest.ThreeThreadsInvoke.
R=henrike@webrtc.org
BUG=

Review URL: https://webrtc-codereview.appspot.com/26819004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7457 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-10-15 14:54:56 +00:00
c732a3e511 Re-enable allmost all base tests.
BUG=3836
R=marpan@google.com

Review URL: https://webrtc-codereview.appspot.com/22989004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7416 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-10-09 22:08:15 +00:00
e30dab77df base/thread_unittest: wrap test was setting current thread to NULL.
This broke unittests following ThreadTest.Wrap

BUG=3836
R=pbos@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/28689004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7413 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-10-09 15:41:40 +00:00
3987b6de50 Fix a problem in Thread::Send.
Previously if thread A->Send is called on thread B, B->ReceiveSends will be called, which enables an arbitrary thread to invoke calls on B while B is wait for A->Send to return. This caused mutliple problems like issue 3559, 3579.
The fix is to limit B->ReceiveSends to only process requests from A.
Also disallow the worker thread invoking other threads.

BUG=3559
R=juberti@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/15089004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7290 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-09-24 17:14:05 +00:00
95705602bd Additional disabled tests in rtc_unittests.
It appears https://review.webrtc.org/27559004/
not enough to get rtc_unittests up and running.
It's currently failing on Linux 32, Linux ASan
and Win SyzyASan bots.

BUG=3836
TBR=henrike@webrtc.org
TEST=Locally passing rtc_unittests on Linux Release
build with asan=1 and lsan=1 in GYP_DEFINES.

Review URL: https://webrtc-codereview.appspot.com/24659004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7242 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-09-19 14:49:37 +00:00
fded02c164 base: disabled several base tests on Mac so that rtc_unittests can be turned back on
BUG=N/A
R=kjellander@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/30449004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7240 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-09-19 13:10:10 +00:00
ba737cba1a Do not require synchronization access on the thread if called from rtc::Thread::WrapCurrent.
The synchronization access is unnecessary for rtc::Thread::WrapCurrent (called from JingleThreadWrapper) since JingleThreadWrapper never calls rtc::Thread::Stop or rtc::Thread::Join. Failing to get the access caused crashes in Chrome since rtc::Thread::Current will be NULL when rtc::Thread::WrapCurrent fails.

rtc::ThreadManager::WrapCurrentThread still requires the synchronization access, since I am not sure if the callers (e.g. the plugin) depends on it.

BUG=crbug/413853
R=juberti@webrtc.org, tommi@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/30429004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@7224 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-09-18 16:45:21 +00:00
e5063b1733 Thread: delete racy API (Release()) and fix racy code (started()).
- Thread::Release() wrote a local variable on the calling thread but read it on
  another thread, with no synchronization.  Happily it has no non-test callers
  so deleting it instead of trying to fix it (see bug for details).
- Thread::started_ similarly was racily being written to; replaced with a
  running_ Event, and hid the accessor except for tests & legacy callers,
  with a note about why it's a bad idea.

webrtc/base patched with:
git diff origin --relative=talk/base | patch -p1 -dwebrtc/base
followed by manual merge of 3 thunks that ran afoul of naming differences
between talk/base and webrtc/base.

BUG=3388
R=andrew@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/14589005

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6236 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-05-23 17:28:50 +00:00
f048872e91 Adds a modified copy of talk/base to webrtc/base. It is the first step in
migrating talk/base to webrtc/base.

BUG=N/A
R=niklas.enbom@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/17479005

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6129 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-05-13 18:00:26 +00:00
e9a604accd Revert 6107 "Adds a modified copy of talk/base to webrtc/base. I..."
This breaks Chromium FYI builds and prevent roll of webrtc/libjingle to Chrome.

http://chromegw.corp.google.com/i/chromium.webrtc.fyi/builders/Win%20Builder/builds/457


> Adds a modified copy of talk/base to webrtc/base. It is the first step in migrating talk/base to webrtc/base.
> 
> BUG=N/A
> R=andrew@webrtc.org, wu@webrtc.org
> 
> Review URL: https://webrtc-codereview.appspot.com/12199004

TBR=henrike@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/14479004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6116 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-05-13 08:15:48 +00:00
2c7d1b39b9 Adds a modified copy of talk/base to webrtc/base. It is the first step in migrating talk/base to webrtc/base.
BUG=N/A
R=andrew@webrtc.org, wu@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/12199004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6107 4adac7df-926f-26a2-2b94-8c16560cd09d
2014-05-12 18:03:09 +00:00