Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ocean.core.Verify #214

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions relnotes/verify.feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
* `ocean.core.Verify`

New module with a single function, `verify`, intended to serve as drop-in
replacement for `assert` to comply to [Sociomantic assert/enforce
policies](https://github.com/sociomantic-tsunami/sociomantic/blob/master/Code/assert-vs-enforce.rst)

It works similar to `enforce` as it throws `Exception` instead of an `Error`
and will remain even when built with `-release`. But it also uses specific
pre-constructed `SanityException` type to indicate importance of the failure.
79 changes: 79 additions & 0 deletions src/ocean/core/Verify.d
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*******************************************************************************

Utility intended as a replacement for `assert` to check for programming
errors and sanity violations in situations when neither removing the check
in -release mode nor bringing down the application by throwing an `Error`
is acceptable.

This module must have as few import dependencies as possible so that it can
be used in place of `assert` freely without introducing cyclic imports.

Copyright: Copyright (c) 2017 sociomantic labs GmbH. All rights reserved

License:
Boost Software License Version 1.0. See LICENSE_BOOST.txt for details.
Alternatively, this file may be distributed under the terms of the Tango
3-Clause BSD License (see LICENSE_BSD.txt for details).

*******************************************************************************/

module ocean.core.Verify;

import ocean.meta.types.Qualifiers : istring;

/*******************************************************************************

Verifies that certain condition is met.

Params:
ok = boolean condition to check
msg = optional exception message

Throws:
SanityException if `ok` condition is `false`.

*******************************************************************************/

public void verify ( bool ok, lazy istring msg = "",
istring file = __FILE__, int line = __LINE__ )
{
static SanityException exc;

if (exc is null)
exc = new SanityException("");

if (!ok)
{
exc.file = file;
exc.line = line;
exc.msg = msg;

throw exc;
}
}

unittest
{
try
{
verify(false);
}
catch (SanityException e) { }

verify(true);
}

/*******************************************************************************

Indicates some internal sanity violation in the app, essentially a less
fatal version of `AssertError`.

*******************************************************************************/

public class SanityException : Exception

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to start a bike shed war, but I wonder why you didn't go for ProgrammingException or if there was any discussion about the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was no specific discussion, I picked the name because I was already using this name in other places to name things with such purpose. I believe it is a more generic and thus correct term than ProgrammingException because cases when we would want to use it may also detect architecture issues or serious violations from trusted input source (like DHT).

{
public this ( istring msg, istring file = __FILE__, int line = __LINE__ )
{
super(msg, file, line);
}
}
42 changes: 8 additions & 34 deletions src/ocean/task/Scheduler.d
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import core.thread;

import ocean.transition;
import ocean.core.Enforce;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is enforce still needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, there are also regular exceptions in scheduler.

import ocean.core.Verify;
import ocean.core.Traits;
import ocean.core.array.Mutation : reverse;
import ocean.io.select.EpollSelectDispatcher;
Expand Down Expand Up @@ -130,16 +131,6 @@ final class Scheduler
/// ditto
private State state = State.Initial;

/***************************************************************************

Thrown instead of `AssertError` when scheduler sanity is violated. Such
issues are almost impossible to reason about if not caught in time
thus we want all sanity checks to remain even in `-release` mode.

***************************************************************************/

private SchedulerSanityException sanity_e;

/***************************************************************************

Thrown when all fibers are busy, task queue is full and no custom
Expand Down Expand Up @@ -278,12 +269,10 @@ final class Scheduler
config.suspended_task_limit
);

this.sanity_e = new SchedulerSanityException;
this.queue_full_e = new TaskQueueFullException;
this.suspend_queue_full_e = new SuspendQueueFullException;

enforce(
this.sanity_e,
verify(
config.task_queue_limit >= config.worker_fiber_limit,
"Must configure task queue size at least equal to worker fiber " ~
"count for optimal task scheduler performance."
Expand Down Expand Up @@ -569,7 +558,7 @@ final class Scheduler
as there is at least one event registered.

Throws:
SchedulerSanityException if there are some active worker fibers
SanityException if there are some active worker fibers
left in the pool by the time there are not events left

***************************************************************************/
Expand Down Expand Up @@ -602,9 +591,9 @@ final class Scheduler
foreach (ref fiber; iterator)
fiber.active_task.kill();

enforce(this.sanity_e, this.fiber_pool.num_busy() == 0);
enforce(this.sanity_e, this.queued_tasks.length() == 0);
enforce(this.sanity_e, this.suspended_tasks.length() == 0);
verify(this.fiber_pool.num_busy() == 0);
verify(this.queued_tasks.length() == 0);
verify(this.suspended_tasks.length() == 0);
}

/***************************************************************************
Expand Down Expand Up @@ -645,7 +634,7 @@ final class Scheduler
task = task to run

Throws:
SchedulerSanityException on attempt to run new task from the very
SanityException on attempt to run new task from the very
same fiber which would result in fiber resetting own state.

***************************************************************************/
Expand Down Expand Up @@ -751,7 +740,7 @@ final class Scheduler
for (auto i = 0; i < current_count; ++i)
{
Task task;
enforce(this.sanity_e, this.suspended_tasks.pop(task));
verify(this.suspended_tasks.pop(task));
if (this.state == State.Shutdown)
task.kill();
else
Expand Down Expand Up @@ -1011,21 +1000,6 @@ public class SuspendQueueFullException : Exception
}
}

/******************************************************************************

Exception class that indicates scheduler internal sanity violation,
for example, worker fiber leak.

******************************************************************************/

private class SchedulerSanityException : Exception
{
this ( )
{
super("Internal sanity violation using the scheduler");
}
}

private void debug_trace ( T... ) ( cstring format, T args )
{
debug ( TaskScheduler )
Expand Down
32 changes: 3 additions & 29 deletions src/ocean/task/internal/FiberPool.d
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module ocean.task.internal.FiberPool;
import core.thread;

import ocean.task.Task;
import ocean.core.Enforce;
import ocean.core.Verify;
import ocean.util.container.pool.ObjectPool;

debug (TaskScheduler)
Expand All @@ -51,15 +51,6 @@ public class FiberPool : ObjectPool!(WorkerFiber)

private size_t stack_size;

/**************************************************************************

Exception object thrown if internal pool state is compromised, should
normally result in application termination.

**************************************************************************/

private FiberPoolSanityException exception;

/**************************************************************************

Constructor
Expand All @@ -75,7 +66,6 @@ public class FiberPool : ObjectPool!(WorkerFiber)
this ( size_t stack_size, size_t limit = 0 )
{
this.stack_size = stack_size;
this.exception = new FiberPoolSanityException;
if (limit > 0)
this.setLimit(limit);
}
Expand All @@ -101,7 +91,7 @@ public class FiberPool : ObjectPool!(WorkerFiber)
auto fiber = super.get(if_missing);
if (fiber is null)
return null;
enforce(this.exception, fiber.state() != Fiber.State.EXEC);
verify(fiber.state() != Fiber.State.EXEC);
return fiber;
}

Expand Down Expand Up @@ -136,25 +126,9 @@ public class FiberPool : ObjectPool!(WorkerFiber)
override protected void resetItem ( Item item )
{
auto fiber = this.fromItem(item);
enforce(
this.exception,
verify(
fiber is Fiber.getThis()
|| fiber.state() == Fiber.State.TERM
);
}
}

/******************************************************************************

Exception class that indicates fiber pool internal sanity violation,
for example, trying to recycle fiber that is still running.

******************************************************************************/

private class FiberPoolSanityException : Exception
{
this ( )
{
super("Internal sanity violation using fiber pool");
}
}