fiorix / go-smpp

SMPP 3.4 Protocol for the Go programming language
MIT License
218 stars 135 forks source link

unexpected PDU ID: Deliver SM #87

Open Boklazhenko opened 4 years ago

Boklazhenko commented 4 years ago

Hello, thx for you job.

i have this problem.

i receiving response error - unexpected PDU ID: Deliver SM

there is source

func (t *Transmitter) submitMsg(sm *ShortMessage, p pdu.Body, dataCoding uint8) (*ShortMessage, error) {
    f := p.Fields()
    f.Set(pdufield.SourceAddr, sm.Src)
    f.Set(pdufield.DestinationAddr, sm.Dst)
    f.Set(pdufield.ShortMessage, sm.Text)
    f.Set(pdufield.RegisteredDelivery, uint8(sm.Register))
    // Check if the message has validity set.
    if sm.Validity != time.Duration(0) {
        f.Set(pdufield.ValidityPeriod, convertValidity(sm.Validity))
    }
    f.Set(pdufield.ServiceType, sm.ServiceType)
    f.Set(pdufield.SourceAddrTON, sm.SourceAddrTON)
    f.Set(pdufield.SourceAddrNPI, sm.SourceAddrNPI)
    f.Set(pdufield.DestAddrTON, sm.DestAddrTON)
    f.Set(pdufield.DestAddrNPI, sm.DestAddrNPI)
    f.Set(pdufield.ESMClass, sm.ESMClass)
    f.Set(pdufield.ProtocolID, sm.ProtocolID)
    f.Set(pdufield.PriorityFlag, sm.PriorityFlag)
    f.Set(pdufield.ScheduleDeliveryTime, sm.ScheduleDeliveryTime)
    f.Set(pdufield.ReplaceIfPresentFlag, sm.ReplaceIfPresentFlag)
    f.Set(pdufield.SMDefaultMsgID, sm.SMDefaultMsgID)
    f.Set(pdufield.DataCoding, dataCoding)
    resp, err := t.do(p)
    if err != nil {
        return nil, err
    }
    sm.resp.Lock()
    sm.resp.p = resp.PDU
    sm.resp.Unlock()
    if resp.PDU == nil {
        return nil, fmt.Errorf("unexpected empty PDU")
    }
    if id := resp.PDU.Header().ID; id != pdu.SubmitSMRespID {
        return sm, fmt.Errorf("unexpected PDU ID: %s", id)
    }
    if s := resp.PDU.Header().Status; s != 0 {
        return sm, s
    }
    return sm, resp.Err
}

may be problem, that outgoing sequence id has same sequence id that and incoming Deliver SM

func (t *Transmitter) do(p pdu.Body) (*tx, error) {
    t.cl.Lock()
    notbound := t.cl.client == nil
    t.cl.Unlock()
    if notbound {
        return nil, ErrNotBound
    }
    if t.cl.WindowSize > 0 {
        inflight := uint(atomic.AddInt32(&t.tx.count, 1))
        defer func(t *Transmitter) { atomic.AddInt32(&t.tx.count, -1) }(t)
        if inflight > t.cl.WindowSize {
            return nil, ErrMaxWindowSize
        }
    }
    rc := make(chan *tx, 1)
    seq := p.Header().Seq
    t.tx.Lock()
    t.tx.inflight[seq] = rc
    t.tx.Unlock()
    defer func() {
        t.tx.Lock()
        delete(t.tx.inflight, seq)
        t.tx.Unlock()
    }()
    err := t.cl.Write(p)
    if err != nil {
        return nil, err
    }
    select {
    case resp := <-rc:
        if resp.Err != nil {
            return nil, resp.Err
        }
        return resp, nil
    case <-t.cl.respTimeout():
        return nil, ErrTimeout
    }
}

may be here deleting happen later

defer func() {
        t.tx.Lock()
        delete(t.tx.inflight, seq)
        t.tx.Unlock()
    }()

sorry for my bad english, can you help ? thank you

fiorix commented 4 years ago

Haven't looked at this in a while. Don't remember having a race condition like this.