541126 - IPDL: Check that protocol graph induced on (directed) manager relation is acyclic

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I'm not really sure what would happen if it were cyclic, currently; probably send ipdlc into an infinite loop.  Pretty easy to check, but also not a high priority bug.

Comment 1

[Thomas Lee]

Cyclic manager/manages relations result in a stack overflow error in $srcdir/ipc/ipdl/ipdl/lower.py (specifically, _FindFriends.walkDownTheProtocolTree)

Patch in the works.

Comment 2

[Thomas Lee]

Attachment: Naive cycle detection patch, revision #1

Attached patch implements a naive cycle detection algorithm. Example error message output:

/home/tom/opensource/electrolysis.hg/ipc/ipdl/test/ipdl/ok/cyclecheck_Parent.ipdl:3: error: cycle(s) detected in manager/manages heirarchy: `cyclecheck_Parent -> cyclecheck_Child -> cyclecheck_Grandchild -> cyclecheck_Parent'

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Hello Thomas, wanted to pass on a couple of practical notes.  First, when you're ready to have a patch reviewed for committing, click "Details" next to the patch, find the "review" row next to your diff, set the dropdown to "?" and set the requestee appropriately.  (For me, you can just enter ":cjones".)  Second, there's an informal "Mozilla patch style" that most reviewers expect; you can emit patches in this format by setting the following in your ~/.hgrc file

[diff]
git = 1
showfunc = 1

[defaults]
diff = -p -U 8
qdiff = -p -U 8

and then generating your patch with

$ hg diff

(or |$hg qdiff| if you're using mercurial queues.)

Thanks for the patch!

Comment 4

[Thomas Lee]

Attachment: Naive cycle detection patch, revision #2

Just updated my hgrc as per your recommendations. New patch should reflect it.

Shall push it through to you for review.

Thanks yet again!

Comment 5

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 423760 [details] [diff] [review]
Naive cycle detection patch, revision #2

Looks good.

Thanks again for the patch!

Comment 6

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Pushed http://hg.mozilla.org/projects/electrolysis/rev/4cdebce7020a

Comment 7

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

http://hg.mozilla.org/mozilla-central/rev/4f4a912c36f0