Merge pull request #132 from gostor/fix/remove-dead-sessions

fix: remove dead sessions from target session list

pkg/port/iscsit/conn.go

@@ -88,7 +88,8 @@ type iscsiConnection struct {
 	rxTask *iscsiTask
 	txTask *iscsiTask
 
-	readLock *sync.RWMutex
+	readLock    *sync.RWMutex
+	cleanupOnce sync.Once
 }
 
 type taskState int

pkg/port/iscsit/iscsid.go

@@ -357,13 +357,9 @@ func (s *ISCSITargetDriver) handler(events byte, conn *iscsiConnection) {
 			s.rxHandler(conn)
 			if conn.state == CONN_STATE_CLOSE {
 				log.Warningf("iscsi connection[%d] closed", conn.cid)
+				s.removeConnectionFromSession(conn)
 				conn.close()
-				IPMutex.Lock()
-				remoteIP := strings.Split(conn.conn.RemoteAddr().String(), ":")[0]
-				if CurrentHostIP == remoteIP {
-					CurrentHostIP = ""
-				}
-				IPMutex.Unlock()
+				s.clearHostIP(conn)
 			}
 		}()
 	}
@@ -373,13 +369,25 @@ func (s *ISCSITargetDriver) handler(events byte, conn *iscsiConnection) {
 	}
 	if conn.state == CONN_STATE_CLOSE {
 		log.Warningf("iscsi connection[%d] closed", conn.cid)
+		s.removeConnectionFromSession(conn)
 		conn.close()
-		IPMutex.Lock()
-		remoteIP := strings.Split(conn.conn.RemoteAddr().String(), ":")[0]
-		if CurrentHostIP == remoteIP {
-			CurrentHostIP = ""
-		}
-		IPMutex.Unlock()
+		s.clearHostIP(conn)
+	}
+}
+
+func (s *ISCSITargetDriver) clearHostIP(conn *iscsiConnection) {
+	if conn.conn == nil {
+		return
+	}
+	IPMutex.Lock()
+	defer IPMutex.Unlock()
+	addr := conn.conn.RemoteAddr()
+	if addr == nil {
+		return
+	}
+	remoteIP := strings.Split(addr.String(), ":")[0]
+	if CurrentHostIP == remoteIP {
+		CurrentHostIP = ""
 	}
 }
 
@@ -491,7 +499,7 @@ func (s *ISCSITargetDriver) rxHandler(conn *iscsiConnection) {
 		case OpLogoutReq:
 			log.Debug("OpLogoutReq")
 			s.setClientStatus(false)
-			if err := iscsiExecLogout(conn); err != nil {
+			if err := s.iscsiExecLogout(conn); err != nil {
 				log.Warningf("set connection to close")
 				conn.state = CONN_STATE_CLOSE
 			}
@@ -559,7 +567,7 @@ func (s *ISCSITargetDriver) iscsiExecLogin(conn *iscsiConnection) error {
 	return conn.buildRespPackage(OpLoginResp, nil)
 }
 
-func iscsiExecLogout(conn *iscsiConnection) error {
+func (s *ISCSITargetDriver) iscsiExecLogout(conn *iscsiConnection) error {
 	log.Infof("Logout request received from initiator: %v", conn.conn.RemoteAddr().String())
 	cmd := conn.req
 	conn.resp = &ISCSICommand{
@@ -574,6 +582,8 @@ func iscsiExecLogout(conn *iscsiConnection) error {
 		conn.resp.ExpCmdSN = conn.session.ExpCmdSN
 		conn.resp.MaxCmdSN = conn.session.ExpCmdSN + conn.session.MaxQueueCommand
 	}
+	// Session cleanup is deferred to CONN_STATE_CLOSE in handler(),
+	// because the logout response must be sent before the session is removed.
 	IPMutex.Lock()
 	remoteIP := strings.Split(conn.conn.RemoteAddr().String(), ":")[0]
 	if CurrentHostIP == remoteIP {
@@ -1018,7 +1028,7 @@ func (s *ISCSITargetDriver) scsiCommandHandler(conn *iscsiConnection) (err error
 		s.setClientStatus(false)
 		conn.txTask = &iscsiTask{conn: conn, cmd: conn.req, tag: conn.req.TaskTag}
 		conn.txIOState = IOSTATE_TX_BHS
-		iscsiExecLogout(conn)
+		s.iscsiExecLogout(conn)
 	case OpTextReq:
 		err = fmt.Errorf("Cannot handle yet %s", opCodeMap[conn.req.OpCode])
 		log.Error(err)

pkg/port/iscsit/session.go

@@ -330,10 +330,41 @@ func (s *ISCSITargetDriver) LookupISCSISession(tgtName string, iniName string, i
 
 func (s *ISCSITargetDriver) UnBindISCSISession(sess *ISCSISession) {
 	target := sess.Target
+	if target == nil {
+		// Discovery sessions have no target; just release the TSIH.
+		s.ReleaseTSIH(sess.TSIH)
+		return
+	}
 	target.SessionsRWMutex.Lock()
 	defer target.SessionsRWMutex.Unlock()
 	delete(target.Sessions, sess.TSIH)
-	scsi.RemoveITNexus(sess.Target.SCSITarget, sess.ITNexus)
+	if sess.ITNexus != nil {
+		scsi.RemoveITNexus(target.SCSITarget, sess.ITNexus)
+	}
+	s.ReleaseTSIH(sess.TSIH)
+	log.Infof("session %x unbound from target %s", sess.TSIH, target.SCSITarget.Name)
+}
+
+// removeConnectionFromSession removes a connection from its session.
+// If the session has no remaining connections, the session is unbound.
+// This is safe to call concurrently; cleanup runs at most once per connection.
+func (s *ISCSITargetDriver) removeConnectionFromSession(conn *iscsiConnection) {
+	conn.cleanupOnce.Do(func() {
+		sess := conn.session
+		if sess == nil {
+			return
+		}
+
+		sess.ConnectionsRWMutex.Lock()
+		delete(sess.Connections, conn.cid)
+		remaining := len(sess.Connections)
+		sess.ConnectionsRWMutex.Unlock()
+
+		if remaining == 0 {
+			s.UnBindISCSISession(sess)
+		}
+		conn.session = nil
+	})
 }
 
 func (s *ISCSITargetDriver) BindISCSISession(conn *iscsiConnection) error {
@@ -412,7 +443,13 @@ func (s *ISCSITargetDriver) BindISCSISession(conn *iscsiConnection) error {
 		if conn.loginParam.tsih == ISCSI_UNSPEC_TSIH {
 			log.Infof("Session Reinstatement initiator name:%v,target name:%v,ISID:0x%x",
 				conn.loginParam.initiator, conn.loginParam.target, conn.loginParam.isid)
-			newSess, err = s.ReInstatement(existConn.session, conn)
+			if existConn != nil {
+				newSess, err = s.ReInstatement(existConn.session, conn)
+			} else {
+				// Old connection already closed; unbind the stale session and create new
+				s.UnBindISCSISession(existSess)
+				newSess, err = s.NewISCSISession(conn)
+			}
 			if err != nil {
 				return err
 			}