1

Combining functions will crash the application without any error. Even surrounding with try-catch will not help.

Details

In an application, I have the ability to specify filters for data being processed. It is working fine when I have just one filter, but when there are multiple filters combined with || operator, the application (asp.net core 3.1) just stop debugging without any exceptions thrown.

Example test that will not pass, not fail, but just crash without any exception:

[Fact]
public void ShouldFilterWithFilterGroup()
{
    // Arrange
    var filters = new List<Func<object, bool>>();
    filters.Add(x => true);
    filters.Add(x => true);

    Func<object, bool> filterFunction = null;
    foreach (var item in filters)
    {
        if (filterFunction == null)
        {
            filterFunction = x => item(x);
        }
        else
        {
            filterFunction = x => filterFunction(x) || item(x);
        }
    }

    // Act
    var filterResult = filterFunction(null); // Test just stops here without throwing any exception

    // Assert
    Assert.True(filterResult);
}
Trevor
  • 7,777
  • 6
  • 31
  • 50
Merdan Gochmuradov
  • 137
  • 1
  • 11
  • How can we replicate this behavior? – Trevor Oct 08 '20 at 14:20
  • 2
    It is StackOverflow exception. – Berkay Yaylacı Oct 08 '20 at 14:22
  • 3
    there is a possibility to have non ending recurrence, check first what function is inside of filterFunction – Filip Kováč Oct 08 '20 at 14:23
  • 2
    you defined filterfunctions to call filterfunction so you end up in a recursion. – Ralf Oct 08 '20 at 14:24
  • 1
    `bool filterfunction( object x ) { return filterfunction(x) || item(x) ;}` would be the equivalent, no? – Fildor Oct 08 '20 at 14:24
  • 1
    @Berkay and @Fildor are correct, this is a stack overflow exception as the `filterFunction` calls back into itself recursively due to the way it's constructed in the else branch of the foreach. – asawyer Oct 08 '20 at 14:29
  • 2
    So you have other answers on why this breaks, but doing it correctly is actually easier than this is anyway. You can just do `var filterFunction = filters.Aggregate((a,b)=> x => a(x) || b(x));` – Servy Oct 08 '20 at 14:32
  • Merdan: Does it matter to you if you actually create a combining function or do you just care for each function to be evaluated and the results ||ed (like in nvoigt's answer)? – Fildor Oct 08 '20 at 14:44
  • 1
    @Fildor Yes, I needed to create a new function, So I used `.Aggregate()` function as @Servy suggested. Thanks to you all for your help! – Merdan Gochmuradov Oct 09 '20 at 00:41

2 Answers2

0

My guess is that you have an infinite loop. filterFunction will call itself without any exit condition. This should cause a stackoverflow exception eventually.

To fix it try assigning the filterfunction to a local variable:

var localFunction = filterFunction;
filterFunction = x => localFunction(x) || item(x);
JonasH
  • 28,608
  • 2
  • 10
  • 23
  • As far as I am aware the c# compiler does not support tail call optimizations. – asawyer Oct 08 '20 at 14:30
  • @asawyer Can you say the same of all of the JITters? (I don't actually know if any/which ones support it) – Servy Oct 08 '20 at 14:35
  • @asawyer you are right that the the compiler does not have tail call optimization. I still think it is *allowed* to do it. In any case, it is probably not relevant for this case, so removed mentioning it. – JonasH Oct 08 '20 at 14:38
  • @Servy I don't know to be honest. I haven't kept up with jitter changes over the years. I imagine some will under some conditions but I don't even know where to start looking for the documentation on it. This question has some interesting reading on the subject but the answers are a few years old and may be out of date now https://stackoverflow.com/questions/491376/why-doesnt-net-c-optimize-for-tail-call-recursion – asawyer Oct 08 '20 at 14:41
0

Yes, you have a recursion. Don't do it manually, there already is a method to check all of them with an "or":

var result = filters.Any(filter => filter(input));

If you actually need a single function to pass around, you can do that, too:

Func<object, bool> filterGroup = (input) => filters.Any(filter => filter(input));
nvoigt
  • 75,013
  • 26
  • 93
  • 142
  • This is evaluating every function, not creating a new function who's results is the OR of each of these functions. – Servy Oct 08 '20 at 14:36
  • In that case this would only be "wrong" if OP cared. We don't know that. – Fildor Oct 08 '20 at 14:38
  • @Fildor Well that's what they're asking a question about doing. If you want to assert that they don't need to do what they asked about, you would need to demonstrate that that's the case, rather than just assuming it. – Servy Oct 08 '20 at 14:39
  • That's the point: I am not assuming anything. I am open to the possibility of a little x-y component in the question. – Fildor Oct 08 '20 at 14:41
  • 1
    @Servy If you really think a new function is required, we could make it `Func filterGroup = (input) => filters.Any(filter => filter(input));` but that's not the real problem here. – nvoigt Oct 08 '20 at 14:43
  • @Fildor But you *are* assuming something. You're assuming that the OP isn't actually interested in what they said they were interested in. That's an assertion that requires evidence. The burden is not on me to prove that the OP actually wants to know the answer to the question that they asked. – Servy Oct 08 '20 at 14:45
  • That's why I just asked OP to clarify :D – Fildor Oct 08 '20 at 14:46
  • @nvoigt Well, it *is* what the question says the real problem is though. It's pretty explicit about it. Again, if you want to assert that the question isn't asking what it says it's asking, that's an assertion that requires evidence. – Servy Oct 08 '20 at 14:46
  • @Fildor So if/when the OP changes their question to a different question, *then* an answer to that different question may become suitable. But not before. – Servy Oct 08 '20 at 14:46
  • Servy. We're not trying to torture you with answers to questions that haven't been asked (on purpose). – Fildor Oct 08 '20 at 14:48
  • In fact, if you really want to be nitpicky, there actually isn't a question asked _at all_. – Fildor Oct 08 '20 at 14:51