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.

Sign up for GitHub

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

Jump to bottom

Daedalus IPC working intermittently on windows #1036

Closed
KtorZ opened this issue Nov 14, 2019 · 2 comments
Closed

Daedalus IPC working intermittently on windows #1036

KtorZ opened this issue Nov 14, 2019 · 2 comments
Assignees
@rvl @KtorZ
Milestone
Usability & Compa...

Comments

@KtorZ
Copy link
Member

KtorZ commented Nov 14, 2019 •
edited by rvl
Loading

Release Operating System Cause
v2019-11-12 Windows Environment v Code

Context

On some machines, Daedalus remains stuck to the splash screen after starting the wallet backend. From looking at the logs, it seems that it never actually receives a Started message from the wallet backend IPC server.

This is somewhat strange and suggest that it'd take too long for Daedalus to put their listen handler on the process in such a way that they would miss the initial Started message now sent very early by the wallet backend.

Steps to Reproduce

  1. Start Daedalus on Windows
  2. Wait a few seconds and see if stuck on the splash screen
  3. Retry until getting stuck
  4. When stucked, look at the wallet backend log and see if Jörmungandr has started

Expected behavior

  1. Daedalus doesn't remain stuck and can advance their initial state-machine to the next step

Actual behavior

  1. It does

Resolution Plan

  • Retry sending Started until we receive a first sign of life for Daedalus (i.e. a QueryPort request)
  • Rather than starting the send and receive async tasks together, run the message write and read functions in turn.

PR

Number Base
#1037
#1044 master

QA

  1. Get a Windows build of Daedalus Testnet Balance check with this fix.
  2. Test in both "cold" and "warm" cases (relating to whether jormungandr has completed the bootstrap phase). The issue seems to happen more often in the "warm" case.
  3. Check for presence of "Started" and "ReplyPort" in Daedalus log. This means that the IPC process completed. Alternatively, look for the wallet api port in the connection status screen.
@KtorZ KtorZ self-assigned this Nov 14, 2019
@KtorZ KtorZ mentioned this issue Nov 14, 2019
Merged
2 tasks
iohk-bors bot added a commit that referenced this issue Nov 14, 2019
@iohk-bors @KtorZ
Merge #1037
f425623 
1037: Fix DaedalusIPC race-condition when sending 'Started' too quickly r=KtorZ a=KtorZ

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

#1036 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] I have artificially reproduced the issue in our mock-daedalus and saw it fails (wait infinitely)
- [x] I have added some retry logic to re-send `Started` every second if Daedalus has shown any sign of life.


# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: KtorZ <[email protected]>
iohk-bors bot added a commit that referenced this issue Nov 14, 2019
@iohk-bors @KtorZ
Merge #1037
02789c9 
1037: Fix DaedalusIPC race-condition when sending 'Started' too quickly r=KtorZ a=KtorZ

# Issue Number

<!-- Put here a reference to the issue this PR relates to and which requirements it tackles -->

#1036 

# Overview

<!-- Detail in a few bullet points the work accomplished in this PR -->

- [x] I have artificially reproduced the issue in our mock-daedalus and saw it fails (wait infinitely)
- [x] I have added some retry logic to re-send `Started` every second if Daedalus has shown any sign of life.


# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: KtorZ <[email protected]>
@rvl
Copy link
Contributor

rvl commented Nov 15, 2019 •
edited
Loading

Sending IPC messages down the ipc channel on Windows seems to block on sending the 3rd message. From that point on the daedalusIPC function is stuck.

I have added a test case on the rvl/1036/daedalus-ipc-race branch.

@KtorZ KtorZ added this to the Usability & Compatibility milestone Nov 15, 2019
@KtorZ KtorZ changed the title Race condition in DaedalusIPC: not receiving 'Started' Race condition in DaedalusIPC: not receiving 'Started' nor 'ReplyPort' Nov 15, 2019
@KtorZ KtorZ changed the title Race condition in DaedalusIPC: not receiving 'Started' nor 'ReplyPort' Daedalus IPC working intermittently on windows Nov 15, 2019
@rvl rvl mentioned this issue Nov 18, 2019
Merged
2 tasks
iohk-bors bot added a commit that referenced this issue Nov 18, 2019
@iohk-bors @rvl
Merge #1044
80bad70 
1044: Fix DaedalusIPC startup issue on Windows r=piotr-iohk a=rvl

Relates to #1036 

# Overview

It's not possible to concurrently read and write to Windows named pipes. One of the operations will block until the other completes.

The order which this happens in determines whether or not Daedalus receives a `ReplyPort` message on Windows.
 
This PR changes `daedalusIPC` to run sends and receives sequentially. It will fix some of the "Waiting for network..." backend connection issues on Daedalus Windows.

# Comments

- [x] Tested in Daedalus on Windows
- [x] Tested in Daedalus on Linux

Later - We should simplify the message scheme so that there is a single message `WalletApiPort <int>`, and no unnecessary request/response.


Co-authored-by: Rodney Lorrimar <[email protected]>
rvl added a commit that referenced this issue Nov 18, 2019
@rvl
DaedalusIPC: Add integration test for #1036
c5e3817
iohk-bors bot added a commit that referenced this issue Nov 18, 2019
@iohk-bors @rvl
Merge #1044
b2804af 
1044: Fix DaedalusIPC startup issue on Windows r=rvl a=rvl

Relates to #1036 

# Overview

It's not possible to concurrently read and write to Windows named pipes. One of the operations will block until the other completes.

The order which this happens in determines whether or not Daedalus receives a `ReplyPort` message on Windows.
 
This PR changes `daedalusIPC` to run sends and receives sequentially. It will fix some of the "Waiting for network..." backend connection issues on Daedalus Windows.

# Comments

- [x] Tested in Daedalus on Windows
- [x] Tested in Daedalus on Linux

Later - We should simplify the message scheme so that there is a single message `WalletApiPort <int>`, and no unnecessary request/response.


Co-authored-by: Rodney Lorrimar <[email protected]>
@rvl rvl self-assigned this Nov 19, 2019
@piotr-iohk
Copy link
Contributor

lgtm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rvl rvl

@KtorZ KtorZ

Labels
None yet
Projects
None yet
Milestone
Usability & Compatibility
Development

No branches or pull requests
3 participants
@rvl @KtorZ @piotr-iohk