Frage

I am using goroutines/channels to check if list of urls are reachable. Here is my code. This seems to always return true. Why is the timeout case not getting executed? The goal is to return false even if one of the urls is not reachable

import "fmt"
import "time"

func check(u string) bool {
    time.Sleep(4 * time.Second)
    return true
}

func IsReachable(urls []string) bool {

    ch := make(chan bool, 1)
    for _, url := range urls {
        go func(u string) {
            select {
            case ch <- check(u):
            case <-time.After(time.Second):
                ch<-false
            }
        }(url)
    }
    return <-ch
}
func main() {
    fmt.Println(IsReachable([]string{"url1"}))
}
War es hilfreich?

Lösung

check(u) will sleep in the current goroutine, i.e. the one that's running func. The select statement is only run properly once it returns, and by that time, both branches are runnable and the runtime can pick whichever one it pleases.

You can solve it by running check inside yet another goroutine:

package main

import "fmt"
import "time"

func check(u string, checked chan<- bool) {
    time.Sleep(4 * time.Second)
    checked <- true
}

func IsReachable(urls []string) bool {

    ch := make(chan bool, 1)
    for _, url := range urls {
        go func(u string) {
            checked := make(chan bool)
            go check(u, checked)
            select {
            case ret := <-checked:
                ch <- ret
            case <-time.After(1 * time.Second):
                ch <- false
            }
        }(url)
    }
    return <-ch
}
func main() {
    fmt.Println(IsReachable([]string{"url1"}))
}

It seems you want to check reachability of a set of URLs, and return true if one of them is available. If the timeout is long compared to the time it takes to spin up a goroutine, you could simplify this by having just one timeout for all URLs together. But we need to make sure that the channel is large enough to hold the answers from all checks, or the ones that don't "win" will block forever:

package main

import "fmt"
import "time"

func check(u string, ch chan<- bool) {
    time.Sleep(4 * time.Second)
    ch <- true
}

func IsReachable(urls []string) bool {
    ch := make(chan bool, len(urls))
    for _, url := range urls {
        go check(url, ch)
    }
    time.AfterFunc(time.Second, func() { ch <- false })
    return <-ch
}
func main() {
    fmt.Println(IsReachable([]string{"url1", "url2"}))
}

Andere Tipps

The reason this always returns true is you are calling check(u) within your select statement. You need to call it within a go routine and then use a select to either wait for the result or timeout.

In case you want to check the reachability of multiple URLs in parallel you need to restructure your code.

First create a function which checks the reachability of one URL:

func IsReachable(url string) bool {
    ch := make(chan bool, 1)
    go func() { ch <- check(url) }()
    select {
    case reachable := <-ch:
        return reachable
    case <-time.After(time.Second):
        // call timed out
        return false
    }
}

Then call this function from a loop:

urls := []string{"url1", "url2", "url3"}
for _, url := range urls {
    go func() { fmt.Println(IsReachable(url)) }()
}

Play

change the line

ch := make(chan bool, 1)

to

ch := make(chan bool)

You did open a asynchronous (= non blocking) channel, but you need a blocking channel to get it work.

The result of true being returned here is deterministic in this scenario, it's not a random one picked up by the runtime, because there's only true value available (however long it may take for it to become available!) being sent into the channel, the false result would never be available for the channel since the time.After() call statement would never get the chance to be executed in the first place!

In this select, the first executable line it sees is check(u) call, not the channel sending call in the first case branch, or any other call at all! And it's only after this first check(u) execution has returned here, would select branch cases get checked and called upon, by which point, the value of true is already to be pushed into the first branch case channel, so no channel blocking here to the select statement, the select can fulfil its purpose promptly here without needing to check its remaining branch cases!

so looks like it's the use of select here that wouldn't seem quite correct in this scenario.

the select branch cases are supposed to listen to channel sending and receiving values directly, or optionally with a default to escape the blocking when necessary.

so the fix is as some people pointed out here already, putting the long running task or process into a separate goroutine, and have it send result into channel, and then in the main goroutine (or whichever other routine that needs that value off the channel), use the select branch cases to either listen on that specific channel for a value, or on the channel provided by the time.After(time.Second) call.

Basically, this line: case ch <- check(u) is correct in the sense of sending a value into a channel, but it's just not for its intended use (i.e. blocking this branch case), because the case channel<- is not being blocked there at all (the time check(u) spends on is all happening before the channel gets involved), since in a separate goroutine, aka, the main one: return <-ch, it's already ready to read that value whenever it gets pushed through. That is why time.After() call statement in the second case branch would never even get a chance to be evaluated, in the first instance!

see this example for a simple solution, ie. the correct use of a select in conjunction of separate goroutines: https://gobyexample.com/timeouts

In case it's useful, here's a generalised version of @Thomas 's answer, much simplified by @mh-cbon

func WithTimeout(delegate func() interface{}, timeout time.Duration) (ret interface{}, ok bool) {
    ch := make(chan interface{}, 1) // buffered
    go func() { ch <- delegate() }()
    select {
    case ret = <-ch:
        return ret, true
    case <-time.After(timeout):
    }
    return nil, false
}

Then you can call to 'timeout' any function

if value,ok := WithTimeout(myFunc, time.Second); ok {
    // returned
} else {
    // didn't return
}

Call like this to wait for a channel

if value,ok := WithTimeout(func()interface{}{return <- inbox}, time.Second); ok {
    // returned
} else {
    // didn't return
}

Like this to try sending

_,ok = WithTimeout(func()interface{}{outbox <- myValue; return nil}, time.Second)
    if !ok{...
Lizenziert unter: CC-BY-SA mit Zuschreibung
Nicht verbunden mit StackOverflow
scroll top