check close before send delay command to hyperstart chan

[gnawux]

see http://ci.hypercontainer.io:8080/job/hyperd-auto/330/consoleFull

Signed-off-by: Wang Xu gnawux@gmail.com

[gnawux]

however, I am not sure should I consider the chan might block at some case.

[laijs]

defer is called when the exit of the function, not when the exit of the scope

[gnawux]

It's an anonymous func....

[laijs]

Ok...

[laijs]

it seams the lock is also needed in the handleMsgFromHyperstart()

[gnawux]

			h.ctlChan <- &hyperstartCmd{Code: res.Code, retMsg: res.Message}

this one?

[laijs]

yes

[laijs]

an error is needed to be sent to cmd.result

[gnawux]

looks so, will add it.

[laijs]

however, I am not sure should I consider the chan might block at some case.

it is a possible, and cause deadlock.
it seems defer + recovery() proper.

[gnawux]

�I don't like the taste of recover().... ok, it's a rare case....

[laijs]

one of the ways to avoid the deadlock, but I don't like it either.

diff --git a/hyperstart/libhyperstart/json.go b/hyperstart/libhyperstart/json.go
index 852a690..76ffbf0 100644
--- a/hyperstart/libhyperstart/json.go
+++ b/hyperstart/libhyperstart/json.go
@@ -82,6 +82,13 @@ func (h *jsonBasedHyperstart) Log(level hlog.LogLevel, args ...interface{}) {
 }

 func (h *jsonBasedHyperstart) Close() {
+       go func() {
+               for cmd := range h.ctlChan {
+                       if cmd.Code != hyperstartapi.INIT_ACK && cmd.Code != hyperstartapi.INIT_ERROR {
+                               cmd.result <- fmt.Errorf("hyperstart closed")
+                       }
+               }
+       }()
        h.Lock()
        defer h.Unlock()
        if !h.closed {
@@ -97,11 +104,6 @@ func (h *jsonBasedHyperstart) Close() {
                close(h.ctlChan)
                close(h.streamChan)
                close(h.processAsyncEvents)
-               for cmd := range h.ctlChan {
-                       if cmd.Code != hyperstartapi.INIT_ACK && cmd.Code != hyperstartapi.INIT_ERROR {
-                               cmd.result <- fmt.Errorf("hyperstart closed")
-                       }
-               }
                h.closed = true
        }
 }

[gnawux]

@laijs I think you could close this and push a new PR

[gnawux]

@laijs updated

[gnawux]

I am considering should I use recover to avoid a lock in a hot path

h.ctlChan <- &hyperstartCmd{Code: res.Code, retMsg: res.Message}

[laijs]

it seams better to avoid the lock in handleMsgFromHyperstart()

[gnawux]

ok, same idea. I will use the ugly recover instead.

[gnawux]

updated again, with recover instead of lock