[jira] [Commented] (MESOS-5886) FUTURE_DISPATCH may react on irrelevant dispatch.
[
https://issues.apache.org/jira/browse/MESOS-5886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16395580#comment-16395580
]
Andrei Budnik commented on MESOS-5886:
--
If a class contains methods with the same signature, we can add one extra
argument of a unique type for each of these methods. We can introduce a special
type `Tag` for extra arguments:
{code:java}
struct None {};
template
struct Tag { Tag(const None&) {} };
class Example : public Process {
public:
void m1(int, Tag<1> tag = None()) {}
void m2(int, Tag<2> tag = None()) {}
void m3(int, Tag<3> tag = None()) {}
};{code}
It looks like current implementation of
[`dispatch()`|https://github.com/apache/mesos/blob/8adb5fcb1f6c451bc9ad7ecdc6e39bc170fdcd65/3rdparty/libprocess/include/process/dispatch.hpp#L209-L256]
supports arguments with a default value, so a user of `dispatch()` doesn't
need to care about the extra tag argument.
Note, that this approach fixes the issue only for non-virtual methods. For more
details, see [On fixing
FUTURE_DISPATCH|https://docs.google.com/document/d/1d4nYfIWuTGtvHObolyzCwBttU75gxbKSvyISjHQ76Nw]
document.
> FUTURE_DISPATCH may react on irrelevant dispatch.
> -
>
> Key: MESOS-5886
> URL: https://issues.apache.org/jira/browse/MESOS-5886
> Project: Mesos
> Issue Type: Bug
>Affects Versions: 1.1.2, 1.2.1, 1.3.0, 1.4.0
>Reporter: Alexander Rukletsov
>Priority: Major
> Labels: mesosphere, tech-debt, tech-debt-test
>
> [{{FUTURE_DISPATCH}}|https://github.com/apache/mesos/blob/e8ebbe5fe4189ef7ab046da2276a6abee41deeb2/3rdparty/libprocess/include/process/gmock.hpp#L50]
> uses
> [{{DispatchMatcher}}|https://github.com/apache/mesos/blob/e8ebbe5fe4189ef7ab046da2276a6abee41deeb2/3rdparty/libprocess/include/process/gmock.hpp#L350]
> to figure out whether a processed {{DispatchEvent}} is the same the user is
> waiting for. However, comparing {{std::type_info}} of function pointers is
> not enough: different class methods with same signatures will be matched.
> Here is the test that proves this:
> {noformat}
> class DispatchProcess : public Process
> {
> public:
> MOCK_METHOD0(func0, void());
> MOCK_METHOD1(func1, bool(bool));
> MOCK_METHOD1(func1_same_but_different, bool(bool));
> MOCK_METHOD1(func2, Future(bool));
> MOCK_METHOD1(func3, int(int));
> MOCK_METHOD2(func4, Future(bool, int));
> };
> {noformat}
> {noformat}
> TEST(ProcessTest, DispatchMatch)
> {
> DispatchProcess process;
> PID pid = spawn();
> Future future = FUTURE_DISPATCH(
> pid,
> ::func1_same_but_different);
> EXPECT_CALL(process, func1(_))
> .WillOnce(ReturnArg<0>());
> dispatch(pid, ::func1, true);
> AWAIT_READY(future);
> terminate(pid);
> wait(pid);
> }
> {noformat}
> The test passes:
> {noformat}
> [ RUN ] ProcessTest.DispatchMatch
> [ OK ] ProcessTest.DispatchMatch (1 ms)
> {noformat}
> This change was introduced in https://reviews.apache.org/r/28052/.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Commented] (MESOS-5886) FUTURE_DISPATCH may react on irrelevant dispatch.
[
https://issues.apache.org/jira/browse/MESOS-5886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16113692#comment-16113692
]
Anand Mazumdar commented on MESOS-5886:
---
[~abudnik] Can you find a shepherd for this? I am retargeting this for 1.5.0.
> FUTURE_DISPATCH may react on irrelevant dispatch.
> -
>
> Key: MESOS-5886
> URL: https://issues.apache.org/jira/browse/MESOS-5886
> Project: Mesos
> Issue Type: Bug
>Affects Versions: 1.1.2, 1.2.1, 1.3.0, 1.4.0
>Reporter: Alexander Rukletsov
>Assignee: Andrei Budnik
> Labels: mesosphere, tech-debt, tech-debt-test
>
> [{{FUTURE_DISPATCH}}|https://github.com/apache/mesos/blob/e8ebbe5fe4189ef7ab046da2276a6abee41deeb2/3rdparty/libprocess/include/process/gmock.hpp#L50]
> uses
> [{{DispatchMatcher}}|https://github.com/apache/mesos/blob/e8ebbe5fe4189ef7ab046da2276a6abee41deeb2/3rdparty/libprocess/include/process/gmock.hpp#L350]
> to figure out whether a processed {{DispatchEvent}} is the same the user is
> waiting for. However, comparing {{std::type_info}} of function pointers is
> not enough: different class methods with same signatures will be matched.
> Here is the test that proves this:
> {noformat}
> class DispatchProcess : public Process
> {
> public:
> MOCK_METHOD0(func0, void());
> MOCK_METHOD1(func1, bool(bool));
> MOCK_METHOD1(func1_same_but_different, bool(bool));
> MOCK_METHOD1(func2, Future(bool));
> MOCK_METHOD1(func3, int(int));
> MOCK_METHOD2(func4, Future(bool, int));
> };
> {noformat}
> {noformat}
> TEST(ProcessTest, DispatchMatch)
> {
> DispatchProcess process;
> PID pid = spawn();
> Future future = FUTURE_DISPATCH(
> pid,
> ::func1_same_but_different);
> EXPECT_CALL(process, func1(_))
> .WillOnce(ReturnArg<0>());
> dispatch(pid, ::func1, true);
> AWAIT_READY(future);
> terminate(pid);
> wait(pid);
> }
> {noformat}
> The test passes:
> {noformat}
> [ RUN ] ProcessTest.DispatchMatch
> [ OK ] ProcessTest.DispatchMatch (1 ms)
> {noformat}
> This change was introduced in https://reviews.apache.org/r/28052/.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (MESOS-5886) FUTURE_DISPATCH may react on irrelevant dispatch.
[
https://issues.apache.org/jira/browse/MESOS-5886?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15389299#comment-15389299
]
Benjamin Bannier commented on MESOS-5886:
-
It seem it might be enough to store just a ptr to the member function in the
{{DispatchEvent}} and then do the matching with that. If we'd ever need to
examine the {{type_info}} of the store member, it should be possible to get it
from that ptr as well (currently we do not really use it but for the fuzzy
match you reported here).
> FUTURE_DISPATCH may react on irrelevant dispatch.
> -
>
> Key: MESOS-5886
> URL: https://issues.apache.org/jira/browse/MESOS-5886
> Project: Mesos
> Issue Type: Bug
>Reporter: Alexander Rukletsov
> Labels: mesosphere, tech-debt, tech-debt-test
>
> [{{FUTURE_DISPATCH}}|https://github.com/apache/mesos/blob/e8ebbe5fe4189ef7ab046da2276a6abee41deeb2/3rdparty/libprocess/include/process/gmock.hpp#L50]
> uses
> [{{DispatchMatcher}}|https://github.com/apache/mesos/blob/e8ebbe5fe4189ef7ab046da2276a6abee41deeb2/3rdparty/libprocess/include/process/gmock.hpp#L350]
> to figure out whether a processed {{DispatchEvent}} is the same the user is
> waiting for. However, comparing {{std::type_info}} of function pointers is
> not enough: different class methods with same signatures will be matched.
> Here is the test that proves this:
> {noformat}
> class DispatchProcess : public Process
> {
> public:
> MOCK_METHOD0(func0, void());
> MOCK_METHOD1(func1, bool(bool));
> MOCK_METHOD1(func1_same_but_different, bool(bool));
> MOCK_METHOD1(func2, Future(bool));
> MOCK_METHOD1(func3, int(int));
> MOCK_METHOD2(func4, Future(bool, int));
> };
> {noformat}
> {noformat}
> TEST(ProcessTest, DispatchMatch)
> {
> DispatchProcess process;
> PID pid = spawn();
> Future future = FUTURE_DISPATCH(
> pid,
> ::func1_same_but_different);
> EXPECT_CALL(process, func1(_))
> .WillOnce(ReturnArg<0>());
> dispatch(pid, ::func1, true);
> AWAIT_READY(future);
> terminate(pid);
> wait(pid);
> }
> {noformat}
> The test passes:
> {noformat}
> [ RUN ] ProcessTest.DispatchMatch
> [ OK ] ProcessTest.DispatchMatch (1 ms)
> {noformat}
> This change was introduced in https://reviews.apache.org/r/28052/.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
