Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2344153002:
Made g_lazy_worker_pool leaky (Closed)
DescriptionMade g_lazy_worker_pool leaky to enable tasks to start tasks.
Since the WorkerPool has CONTINUE_ON_SHUTDOWN semantics, tasks are
disallowed from using singletons, since these could get null while the
task is still running. Because of that, it is impossible to start tasks
from within a task.
A leaky singleton on the other hand will never get null again and is
allowed to be used from background threads, fixing the issue.
BUG=648152
Committed: https://crrev.com/64e7172160ea40b922cc52259d6d2fe29a2b3882
Cr-Commit-Position: refs/heads/master@{#419705}
Patch Set 1 #
Messages
Total messages: 22 (13 generated)
The CQ bit was checked by heimbuef@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
heimbuef@google.com changed reviewers: + jochen@chromium.org
PTAL
You'll need to file an base/ OWNER - I'd recommend git blame'ing the file to see who commonly touches it Please also file a tracking bug, and add some more explanation to the CL description as to why you want to change this.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
heimbuef@google.com changed reviewers: + mark@chromium.org
Added mark@ because he's an owner. PTAL
Please follow Jochen’s other suggestions too.
Description was changed from ========== Made g_lazy_worker_pool leaky BUG= ========== to ========== Made g_lazy_worker_pool leaky (recommended by gab@google.com). This will enable Task creation from background threads (so from other tasks for example). BUG=648152 ==========
Description was changed from ========== Made g_lazy_worker_pool leaky (recommended by gab@google.com). This will enable Task creation from background threads (so from other tasks for example). BUG=648152 ========== to ========== Made g_lazy_worker_pool leaky (recommended by gab@google.com). This will enable Task creation from background threads (so from other tasks for example). From gab@google.com: " The issue is that you're using the WorkerPool which has CONTINUE_ON_SHUTDOWN semantics (I.e. not joined on shutdown). As such using singletons is disallowed as they could become null from under a task still running on a unjoined thread during shutdown. The solution is to either: A) Make your singleton/LazyInstance use Leaky traits (ref. discussion). or B) Use SequencedWorkerPool with SKIP_ON_SHUTDOWN semantics. This is all about to become more straightforward with project Lucky Luke (base/task_scheduler), but until then, those are your options." BUG=648152 ==========
On 2016/09/16 14:20:00, Mark Mentovai wrote: > Please follow Jochen’s other suggestions too. Done.
LGTM. Wrap the commit message at 72 characters for maximum git-friendliness. Google for “git commit 72 characters” or similar for lots of opining on the subject.
Description was changed from ========== Made g_lazy_worker_pool leaky (recommended by gab@google.com). This will enable Task creation from background threads (so from other tasks for example). From gab@google.com: " The issue is that you're using the WorkerPool which has CONTINUE_ON_SHUTDOWN semantics (I.e. not joined on shutdown). As such using singletons is disallowed as they could become null from under a task still running on a unjoined thread during shutdown. The solution is to either: A) Make your singleton/LazyInstance use Leaky traits (ref. discussion). or B) Use SequencedWorkerPool with SKIP_ON_SHUTDOWN semantics. This is all about to become more straightforward with project Lucky Luke (base/task_scheduler), but until then, those are your options." BUG=648152 ========== to ========== Made g_lazy_worker_pool leaky to enable tasks to start tasks. Since the WorkerPool has CONTINUE_ON_SHUTDOWN semantics, tasks are disallowed from using singletons, since these could get null while the task is still running. Because of that, it is impossible to start tasks from within a task. A leaky singleton on the other hand will never get null again and is allowed to be used from background threads, fixing the issue. BUG=648152 ==========
Description was changed from ========== Made g_lazy_worker_pool leaky to enable tasks to start tasks. Since the WorkerPool has CONTINUE_ON_SHUTDOWN semantics, tasks are disallowed from using singletons, since these could get null while the task is still running. Because of that, it is impossible to start tasks from within a task. A leaky singleton on the other hand will never get null again and is allowed to be used from background threads, fixing the issue. BUG=648152 ========== to ========== Made g_lazy_worker_pool leaky to enable tasks to start tasks. Since the WorkerPool has CONTINUE_ON_SHUTDOWN semantics, tasks are disallowed from using singletons, since these could get null while the task is still running. Because of that, it is impossible to start tasks from within a task. A leaky singleton on the other hand will never get null again and is allowed to be used from background threads, fixing the issue. BUG=648152 ==========
The CQ bit was checked by heimbuef@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Made g_lazy_worker_pool leaky to enable tasks to start tasks. Since the WorkerPool has CONTINUE_ON_SHUTDOWN semantics, tasks are disallowed from using singletons, since these could get null while the task is still running. Because of that, it is impossible to start tasks from within a task. A leaky singleton on the other hand will never get null again and is allowed to be used from background threads, fixing the issue. BUG=648152 ========== to ========== Made g_lazy_worker_pool leaky to enable tasks to start tasks. Since the WorkerPool has CONTINUE_ON_SHUTDOWN semantics, tasks are disallowed from using singletons, since these could get null while the task is still running. Because of that, it is impossible to start tasks from within a task. A leaky singleton on the other hand will never get null again and is allowed to be used from background threads, fixing the issue. BUG=648152 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Made g_lazy_worker_pool leaky to enable tasks to start tasks. Since the WorkerPool has CONTINUE_ON_SHUTDOWN semantics, tasks are disallowed from using singletons, since these could get null while the task is still running. Because of that, it is impossible to start tasks from within a task. A leaky singleton on the other hand will never get null again and is allowed to be used from background threads, fixing the issue. BUG=648152 ========== to ========== Made g_lazy_worker_pool leaky to enable tasks to start tasks. Since the WorkerPool has CONTINUE_ON_SHUTDOWN semantics, tasks are disallowed from using singletons, since these could get null while the task is still running. Because of that, it is impossible to start tasks from within a task. A leaky singleton on the other hand will never get null again and is allowed to be used from background threads, fixing the issue. BUG=648152 Committed: https://crrev.com/64e7172160ea40b922cc52259d6d2fe29a2b3882 Cr-Commit-Position: refs/heads/master@{#419705} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/64e7172160ea40b922cc52259d6d2fe29a2b3882 Cr-Commit-Position: refs/heads/master@{#419705} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
