update bad value protection for timer value #332

Merged
merged 2 commits into from Jun 4, 2013

Conversation

Projects
None yet
2 participants
Contributor

evanmcc commented Jun 3, 2013

Since there are many paths to spawn a coverage FSM, it would be a lot of code duplication to validate the timeout at each of those entry points. Instead, expand may_start_timeout_timer to guard against more categories of bad input.

Fixes basho/riak_kv#571

cc @jrwest @seancribbs

@jrwest jrwest commented on an outdated diff Jun 4, 2013

src/riak_core_coverage_fsm.erl
@@ -195,6 +197,10 @@ init({test, Args, StateProps}) ->
%% @private
maybe_start_timeout_timer(infinity) ->
ok;
+maybe_start_timeout_timer(Bad) when not is_integer(Bad) ->
+ maybe_start_timeout_timer(?DEFAULT_TIMEOUT);
+maybe_start_timeout_timer(undefined) ->
@jrwest

jrwest Jun 4, 2013

Contributor

this clause is covered by the previous one

@jrwest jrwest commented on the diff Jun 4, 2013

src/riak_core_coverage_fsm.erl
@@ -97,6 +97,8 @@ behaviour_info(callbacks) ->
behaviour_info(_) ->
undefined.
+-define(DEFAULT_TIMEOUT, 60000*8).
@jrwest

jrwest Jun 4, 2013

Contributor

what's the reasoning behind this default? is it just supposed to be greater than all the other timeouts in layers above?

@evanmcc

evanmcc Jun 4, 2013

Contributor

It's the same default that's defined in the client.

@jrwest

jrwest Jun 4, 2013

Contributor

ok cool. just wanted to know its origin. thanks

Contributor

jrwest commented Jun 4, 2013

client_java_verify passes for me w/ these changes

Contributor

jrwest commented Jun 4, 2013

+1

evanmcc merged commit b5b94dd into master Jun 4, 2013

1 check failed

default The Travis CI build failed
Details

russelldb referenced this pull request in basho/riak_kv Jul 26, 2013

Closed

Add timeout to 2i endpoints #610

seancribbs deleted the pevm-timeout-guard branch Apr 1, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment