nullptr in abstraction::ValueHolder<object::Object>::getActualType) while using ValueHolder in evalAlgorithm helper as parameter for the second time

added C-cli P-high T-bug labels

I can't simulate the crash via aql. However, it seems that you don't have to evaluate the auto compose2 = ... line. It's enough to call auto X = compose1->getActualType(); to crash.

changed the description

changed title from {-segfault while algorithm eval (abstraction::ValueHolderobject::Object::getActualType)-} to {+nullptr in abstraction::ValueHolderobject::Object::getActualType) while using ValueHolder in evalAlgorithm helper as parameter for the second time+}

changed the description

Got it. The nullptr is there since you have moved from the value at the time you constructed the value for the formal parameter of mojeFunkce. You have said it is a temporary with the true here:

auto input = std::make_shared<abstraction::ValueHolder<std::string>>(std::string {"neco"}, true);

However, there is a bug still. Even if you accept as const reference the internal creation of return value does not honour the claim of the reference (not a temporary) but looks at the original value (a temporary) and moves still... Working on fixing this.

In the meantime, the worker should probably not create temporaries. Algorithms accept const references anyway (with some exceptions - parse algos - for which you should move the value, as in aql Move - | string::Parse @automaton::Automaton -).

In the meantime, the worker should probably not create temporaries

Agreed. But what do you mean by "in the meantime"? Even if I set the temporary arg to false, it still fails (and you said so as well)

with some exceptions - parse algos - for which you should move the value

Just to be sure: "should move" represents SHOULD or MUST? If this is a MUST then things get complicated by traversing the graph to determine if we MUST move or not.

I'd rather disable all moves just to be sure :-)

Working on fixing this

Thanks! I will backport into the correct branch when done (perhaps I can backport everything from master except the determinize variant registration commit again)

Well, by using not-temporary values we'll get the same behaviour as aql, the evaluator will not move unless explicitly asked to so. What I meant by the bug applies to keeping temporaries (current state) and a UDF that accepts const ref and returns a value.

SHOULD or MUST, not important, without a move, you won't be able to bind parameters.

Could we try with another UDF function? one that accepts a const reference and returns a const reference? With it, it should work.

SHOULD or MUST, not important, without a move, you won't be able to bind parameters.

Okay, I admit I don't know how these internals works.

@travnja3 Any progress on this? It crashed for me at today's lecture.

Also: How can I stop using temporaries if everything core returns (even string::Parse result) after evaluation is a temporary?

mentioned in issue webui-client#91 (closed)

OK I noticed that you have unmerged commit 1172495e in your branch dealing with the first issue. But this code crashes anyway

#include <abstraction/TemporariesHolder.h>
#include <abstraction/ValueHolder.hpp>
#include <ast/command/EvalCommand.h>
#include <common/EvalHelper.h>
#include <ext/exception>
#include <global/GlobalData.h>
#include <iostream>

void test_mwe()
{
    cli::Environment environment;

    std::string str("DFA 0 1\n >A - B\n B C -\n C - D\n D E -\n <E E E\n");

    auto input = std::make_shared<abstraction::ValueHolder<std::string>>(std::move(str), false);
    auto movedInput = abstraction::EvalHelper::evalAlgorithm(environment, "cli::builtin::Move", {}, {input}, abstraction::AlgorithmCategories::AlgorithmCategory::NONE);

    auto automaton = abstraction::EvalHelper::evalAlgorithm(environment, "string::Parse", {"automaton::Automaton"}, {movedInput}, abstraction::AlgorithmCategories::AlgorithmCategory::NONE);
    auto determinize = abstraction::EvalHelper::evalAlgorithm(environment, "automaton::determinize::Determinize", {}, {automaton}, abstraction::AlgorithmCategories::AlgorithmCategory::NONE);

    // now it crashes
    auto compose = abstraction::EvalHelper::evalAlgorithm(environment, "string::Compose", {}, {automaton}, abstraction::AlgorithmCategories::AlgorithmCategory::NONE);
}

int main()
{
    try {
        test_mwe();
    } catch (...) {
        return alib::ExceptionHandler::handle(common::Streams::err);
    }
}

OK, so it fails because after the famous "merge all Determinization registrations commit" 1a4de925. the types of args are not const T& but T. After reverting this commit it works at least on my crashing examples.

~~I find this an ugly hack so we'd be grateful if you were able to find a permanent solution because I no longer see when it should work and when it should not.~~

And it crashes in the webui on a very simple operation that inputs an automaton and then it uses the input to both a) visualize it and to b) Determinize it.

Okay, I still don't understand how this works. However, I looked into the CLI code and found out that I should probably make a non-temporary clone of the algo result in the worker (which is completely reasonable, because I can't move twice out of the temporary). Things seems better now, even without the determinization revert. However, as I stated before

mentioned in commit webui-client@b2cda7d3

mentioned in commit webui-client@c3448d6c

mentioned in merge request webui-client!65 (merged)

nullptr in abstraction::ValueHolder<object::Object>::getActualType) while using ValueHolder in evalAlgorithm helper as parameter for the second time

Child items ...

Activity