1554447 - ERR-CONDUIT-CORE: Error while reading "ids" when pushing a stack with duplicate revisions

Status: RESOLVED FIXED

(Conduit :: moz-phab, defect, P2)

Description

[Jean-Yves Avenard [:jya]]

Can push individual patch or revision:

$ moz-phab submit 3dab63263a92~ 3dab63263a92
Submitting 1 commit for review:
(D32602) 3dab63263a92 Bug 1554438 - P3. Shutdown threads whenever they exist. r?Bas,sotaro,mattwoodrow
Submit to Phabricator (YES/No/Always)?

Updating revision D32602:
3dab63263a92 Bug 1554438 - P3. Shutdown threads whenever they exist. r?Bas,sotaro,mattwoodrow
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
SKIP STAGING Phabricator does not support staging areas for this repository.
Updated an existing Differential revision:
Revision URI: https://phabricator.services.mozilla.com/D32602

Included changes:
M gfx/ipc/GPUParent.cpp

Completed
(D32602) 5860c07e90b9 Bug 1554438 - P3. Shutdown threads whenever they exist. r?Bas,sotaro,mattwoodrow
-> https://phabricator.services.mozilla.com/D32602

jyavenard@jya7980ex /Work/Mozilla/gecko
$ moz-phab submit 3dab63263a92~~ 3dab63263a92
Submitting 1 commit for review:
(D32602) 0e83f15ea589 Bug 1554438 - P2. Only shutdown thread if it was started. r?Bas,sotaro,mattwoodrow
Submit to Phabricator (YES/No/Always)?

Updating revision D32602:
0e83f15ea589 Bug 1554438 - P2. Only shutdown thread if it was started. r?Bas,sotaro,mattwoodrow
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
SKIP STAGING Phabricator does not support staging areas for this repository.
Updated an existing Differential revision:
Revision URI: https://phabricator.services.mozilla.com/D32602

Included changes:
M gfx/webrender_bindings/RenderThread.cpp

Completed
(D32602) 0e83f15ea589 Bug 1554438 - P2. Only shutdown thread if it was started. r?Bas,sotaro,mattwoodrow
-> https://phabricator.services.mozilla.com/D32602

But not a stack:
$ moz-phab submit 3dab63263a925315a81a9af47658220accd23657~3 3dab63263a925315a81a9af47658220accd23657
Submitting 3 commits for review:
(D32602) 3dab63263a92 Bug 1554438 - P3. Shutdown threads whenever they exist. r?Bas,sotaro,mattwoodrow
(D32602) 0e83f15ea589 Bug 1554438 - P2. Only shutdown thread if it was started. r?Bas,sotaro,mattwoodrow
(D32601) 839f80b2a6b3 Bug 1554438 - P1. Don't rely on a live pref to determine order of actions. r?Bas,sotaro
Submit to Phabricator (YES/No/Always)?
ERR-CONDUIT-CORE: Error while reading "ids": Expected a nonempty list, but value is an empty list.

Comment 1

[Jean-Yves Avenard [:jya]]

I attempted to push a smaller stack (some commit ids as mentioned here)

the new P3 patch got exactly the same comment as its parent !

Comment 2

[:glob ✱]

are you able to attach the relevant part of the moz-phab log file? ~/.mozbuild/moz-phab/moz-phab.log.
i'm mostly after the php call stack for that error.

Comment 3

[Jean-Yves Avenard [:jya]]

I did a grep of ids and found it in ~/.mozbuild/moz-phab/moz-phab.log.2019-05-26

Here is an extract:

2019-05-26 12:03:05,286 WARNING Submitting 3 commits for review:
2019-05-26 12:03:05,292 DEBUG Arc stdin: {'attachments': {'reviewers': True}, 'constraints': {'ids': [32601, 32602]}}
2019-05-26 12:03:05,292 DEBUG $ 'c:/Users/jyavenard.mozbuild\moz-phab\arcanist\bin\arc.bat' call-conduit differential.revision.search
2019-05-26 12:03:06,651 DEBUG Waiting for JSON parameters on stdin...
{"error":null,"errorMessage":null,"response":{"data":[{"id":32602,"type":"DREV","phid":"PHID-DREV-b6kraotr2bbeirkwqzmd","fields":{"title":"Bug 1554438 - P2. Only shutdown thread if it was started. r?Bas,sotaro","authorPHID":"PHID-USER-v4w3t4wtiaiqrc2d3ach","status":{"value":"accepted","name":"Accepted","closed":false,"color.ansi":"green"},"repositoryPHID":"PHID-REPO-saax4qdxlbbhahhp2kg5","diffPHID":"PHID-DIFF-dgykp7gy5ctejqosdgds","summary":"It is theorically possible that the WinCompositorWindowThread failed to start. Should this happen, attempting to shut it down will cause shutdown to hang forever.\n\nDepends on D32601","testPlan":"","isDraft":false,"holdAsDraft":false,"dateCreated":1558833629,"dateModified":1558835641,"policy":{"view":"public","edit":"PHID-PROJ-njo5uuqyyq3oijbkhy55"},"bugzilla.bug-id":"1554438"},"attachments":{"reviewers":{"reviewers":[{"reviewerPHID":"PHID-USER-jbjyslult4pxam5s4f6h","status":"added","isBlocking":false,"actorPHID":null},{"reviewerPHID":"PHID-USER-b7l2bar52zjm3qjrdkjb","status":"accepted","isBlocking":false,"actorPHID":"PHID-USER-b7l2bar52zjm3qjrdkjb"},{"reviewerPHID":"PHID-USER-dxkrohpjet6yh65movaa","status":"added","isBlocking":false,"actorPHID":null}]}}},{"id":32601,"type":"DREV","phid":"PHID-DREV-gw6rqn7tgybdqnoqknwz","fields":{"title":"Bug 1554438 - P1. Don't rely on a live pref to determine order of actions. r?Bas,sotaro","authorPHID":"PHID-USER-v4w3t4wtiaiqrc2d3ach","status":{"value":"accepted","name":"Accepted","closed":false,"color.ansi":"green"},"repositoryPHID":"PHID-REPO-saax4qdxlbbhahhp2kg5","diffPHID":"PHID-DIFF-pft7c35th2g4bo4f77et","summary":"The value of the pref may change between start and shutdown.\n\nSo we shouldn't rely on that pref to determine if we need to shutdown on thread started earlier if that pref was true.","testPlan":"","isDraft":false,"holdAsDraft":false,"dateCreated":1558833618,"dateModified":1558835680,"policy":{"view":"public","edit":"PHID-PROJ-njo5uuqyyq3oijbkhy55"},"bugzilla.bug-id":"1554438"},"attachments":{"reviewers":{"reviewers":[{"reviewerPHID":"PHID-USER-jbjyslult4pxam5s4f6h","status":"added","isBlocking":false,"actorPHID":null},{"reviewerPHID":"PHID-USER-b7l2bar52zjm3qjrdkjb","status":"accepted","isBlocking":false,"actorPHID":"PHID-USER-b7l2bar52zjm3qjrdkjb"},{"reviewerPHID":"PHID-USER-dxkrohpjet6yh65movaa","status":"added","isBlocking":false,"actorPHID":null}]}}}],"maps":[],"query":{"queryKey":null},"cursor":{"limit":100,"after":null,"before":null,"order":null}}}
2019-05-26 12:03:06,651 INFO (D32602) 3dab63263a92 Bug 1554438 - P3. Shutdown threads whenever they exist. r?Bas,sotaro,mattwoodrow
2019-05-26 12:03:06,653 INFO (D32602) 0e83f15ea589 Bug 1554438 - P2. Only shutdown thread if it was started. r?Bas,sotaro,mattwoodrow
2019-05-26 12:03:06,654 INFO (D32601) 839f80b2a6b3 Bug 1554438 - P1. Don't rely on a live pref to determine order of actions. r?Bas,sotaro
2019-05-26 12:03:06,661 DEBUG Arc stdin: {'usernames': ['sotaro', 'mattwoodrow', 'bas']}
2019-05-26 12:03:06,661 DEBUG $ 'c:/Users/jyavenard.mozbuild\moz-phab\arcanist\bin\arc.bat' call-conduit user.query
2019-05-26 12:03:08,065 DEBUG Waiting for JSON parameters on stdin...
{"error":null,"errorMessage":null,"response":[{"phid":"PHID-USER-jbjyslult4pxam5s4f6h","userName":"bas","realName":"Bas Schouten","image":"https://d3kxowhw4s8amj.cloudfront.net/file/data/ueuq7fxvptrcxzuvk7m5/PHID-FILE-grbmab3zi5bawbnx2tlu/347b81-alphanumeric_lato-white_B.png-0%2C0%2C0%2C0.png","uri":"https://phabricator.services.mozilla.com/p/bas/","roles":["unverified","approved","activated"]},{"phid":"PHID-USER-b7l2bar52zjm3qjrdkjb","userName":"sotaro","realName":"Sotaro Ikeda","image":"https://d3kxowhw4s8amj.cloudfront.net/file/data/smxvfbtwevaiqav6qmwv/PHID-FILE-x7rql3k5ovmrb4fgpaiy/265582-alphanumeric_aleo-white_S.png-0%2C0%2C0%2C0.png","uri":"https://phabricator.services.mozilla.com/p/sotaro/","roles":["verified","approved","activated"]},{"phid":"PHID-USER-dxkrohpjet6yh65movaa","userName":"mattwoodrow","realName":"Matt Woodrow","image":"https://d3kxowhw4s8amj.cloudfront.net/file/data/fdxgd6fpusl4ozbecmkq/PHID-FILE-utpsx7ktkmmlm5cnl452/48663d-alphanumeric_aleo-white_M.png-0%2C0%2C0%2C0.3.png","uri":"https://phabricator.services.mozilla.com/p/mattwoodrow/","roles":["verified","approved","activated"]}]}
2019-05-26 12:03:09,062 DEBUG Arc stdin: {'attachments': {'reviewers': True}, 'constraints': {'ids': []}}
2019-05-26 12:03:09,062 DEBUG $ 'c:/Users/jyavenard.mozbuild\moz-phab\arcanist\bin\arc.bat' call-conduit differential.revision.search
2019-05-26 12:03:10,874 DEBUG Waiting for JSON parameters on stdin...
{"error":"ERR-CONDUIT-CORE","errorMessage":"ERR-CONDUIT-CORE: Error while reading "ids": Expected a nonempty list, but value is an empty list.","response":null}
2019-05-26 12:03:10,875 ERROR ERR-CONDUIT-CORE: Error while reading "ids": Expected a nonempty list, but value is an empty list.
2019-05-26 12:03:27,676 DEBUG found git repo in c:\Users\jyavenard\Work\Mozilla\gecko
2019-05-26 12:03:27,677 DEBUG $ git.exe config --list
2019-05-26 12:03:27,726 DEBUG core.symlinks=false
core.autocrlf=input
core.fscache=true
color.diff=auto
color.status=auto
color.branch=auto
color.interactive=true
help.format=html
rebase.autosquash=true
http.sslcainfo=C:/Program Files/Git/mingw64/ssl/certs/ca-bundle.crt
http.sslbackend=openssl
diff.astextplain.textconv=astextplain
filter.lfs.clean=git-lfs clean -- %f

Let me know if you need the whole file.

Comment 4

[:glob ✱]

thanks - relevant extract:

2019-05-26 12:03:09,062 DEBUG Arc stdin: {'attachments': {'reviewers': True}, 'constraints': {'ids': []}}
2019-05-26 12:03:09,062 DEBUG $ 'c:/Users/jyavenard.mozbuild\moz-phab\arcanist\bin\arc.bat' call-conduit differential.revision.search
2019-05-26 12:03:10,874 DEBUG Waiting for JSON parameters on stdin...
{"error":"ERR-CONDUIT-CORE","errorMessage":"ERR-CONDUIT-CORE: Error while reading "ids": Expected a nonempty list, but value is an empty list.","response":null}

moz-phab shouldn't be passing in an empty list of revisions.

Comment 5

[:glob ✱]

what's going on is this stack has the same revision on parts 2 and 3:

(D32602) 3dab63263a92 Bug 1554438 - P3. Shutdown threads whenever they exist. r?Bas,sotaro,mattwoodrow
(D32602) 0e83f15ea589 Bug 1554438 - P2. Only shutdown thread if it was started. r?Bas,sotaro,mattwoodrow
(D32601) 839f80b2a6b3 Bug 1554438 - P1. Don't rely on a live pref to determine order of actions. r?Bas,sotaro

moz-phab grabs the information for 2 revisions from a cached response, but doesn't account for duplicates when determining if there are any revisions that we need to query phabricator for.

Comment 6

[:glob ✱]

Attachment: Bug 1554447 - ERR-CONDUIT-CORE: Error while reading "ids" when pushing a stack with duplicate revisions; r=zalun!

• Rewrite get_revisions to avoid duplicated code and checks.
• Don't query Phabricator with an empty list.
• Update tests to pass in real data to get_revisions and clear revision
  mapping cache between tests.

Comment 7

[:glob ✱]

https://github.com/mozilla-conduit/review/commit/b8bf713614b5e2109e854db5d27548d01dba5f0b