Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: PaychGetRestartAfterAddFundsMsg may stuck in forever waiting #8829

Merged
merged 1 commit into from
Jun 9, 2022
Merged

Fix: PaychGetRestartAfterAddFundsMsg may stuck in forever waiting #8829

merged 1 commit into from
Jun 9, 2022

Conversation

zl03jsj
Copy link
Contributor

@zl03jsj zl03jsj commented Jun 9, 2022

TestPaychGetRestartAfterAddFundsMsg may stuck in forever waiting

as following state diagram shows:

stateDiagram-v2
  state "pm.store.WithPendingAddFunds"  as queryMsgFromStore
  Test: TestPaychGetRestartAfterAddFundsMsg
  state "_, mcid2, err := : mgr.GetPaych(ctx, from, to, amt2)"as GetPaych
  waitForAddFundsMsg:channelAccessor.waitForAddFundsMsg
  state "mockPaychAPI.WaitMsg(mcid2..." as WaitMsg
  state "receipt := <- response " as waitReceiptResponse
  concurrency: concurrency operations
  state  "mgr2, err := newManager(...)" as Mgr2
  state "done := make(chan struct{})
pchapi.waitingResponses[mcid] = &waitingResponse{receipt: receipt, done: done}
pchapi.lk.Unlock()
<-done
" as WaitCode
  
  Test --> GetPaych
  GetPaych --> channelAccessor.GetPaych
  channelAccessor.GetPaych --> channelAccessor.addFunds: this is started by mgr1, in another go-routine
  channelAccessor.GetPaych --> mock.close
mock.close --> Mgr2
Mgr2 --> manager.Start
mock.close --> waitReceiptResponse: Msg `mcid2` received receipt,\nthis will unblock the waiting
note left of waitReceiptResponse
            blocked here, waiting for Receipt of `mcid2` to continue
        end note

state concurrency {
  channelAccessor.addFunds --> waitForAddFundsMsg
  waitForAddFundsMsg --> WaitMsg
  WaitMsg --> waitReceiptResponse
waitReceiptResponse --> channelAccessor.mutateChannelInfo: ChannelInfo.AddFundsMsg = nil
channelAccessor.mutateChannelInfo --> store.SaveChannelInfo: actually the message of `mcid2`, is set as complete.
note right of store.SaveChannelInfo 
marked as C
end note
--
manager.Start --> manager.restartPending
manager.restartPending --> queryMsgFromStore
note right of queryMsgFromStore
            read uncompleted `AddFunds` Messages from store, re-waiting receipts on chain
        end note
queryMsgFromStore --> mockPaychAPI.WaitMsg(mcid2,..: run in go-routine
queryMsgFromStore --> mockPaychAPI.receiveMsgResponse(mcid2,...
mockPaychAPI.receiveMsgResponse(mcid2,... --> WaitCode

note right of mockPaychAPI.WaitMsg(mcid2,..
marked as A
end note
note right of WaitCode 
marked as B
end note
}
Loading

the A, B, C are execute asynchronously, their execution order are unpredictable.
if the order is in bad path: C -> B,
in that case, the A(mockPaychAPI.StateWaitMsg of mcid2) would never happen,
the B would blocked by receiveMsgResponse(mcid2, )

mock2.receiveMsgResponse(mcid2, types.MessageReceipt{
ExitCode: 0,
Return: []byte{},
})

waiting <-done forever. the test would fail of timeout.

@zl03jsj zl03jsj requested a review from a team as a code owner June 9, 2022 08:59
Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense. Thanks a lot for the PR!

@magik6k magik6k merged commit 1190050 into filecoin-project:master Jun 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants