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

Prevent deserialization of infinitely-recursive CircuitOperations #4304

Closed
95-martin-orion opened this issue Jul 9, 2021 · 1 comment · Fixed by #4315
Closed

Prevent deserialization of infinitely-recursive CircuitOperations #4304

95-martin-orion opened this issue Jul 9, 2021 · 1 comment · Fixed by #4315
Assignees
Labels
area/serialization area/subcircuits kind/feature-request Describes new functionality triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on

Comments

@95-martin-orion
Copy link
Collaborator

Is your feature request related to a use case or problem? Please describe.
Immutability of CircuitOperations prevents users from accidentally defining self-referencing CircuitOperations, but it is still possible to intentionally construct such an operation.

Describe the solution you'd like
Before we enable the use of CircuitOperations in QCS, we must block infinitely-recursive CircuitOperations from being deserialized. The deserializers should include a check to guard against this.

What is the urgency from your perspective for this issue? Is it blocking important work?
P1 - I need this no later than the next release (end of quarter)

@95-martin-orion 95-martin-orion added the kind/feature-request Describes new functionality label Jul 9, 2021
@95-martin-orion 95-martin-orion self-assigned this Jul 9, 2021
@95-martin-orion
Copy link
Collaborator Author

Part of #3634.

@viathor viathor added area/serialization area/subcircuits triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on labels Jul 9, 2021
CirqBot pushed a commit that referenced this issue Jul 13, 2021
Fixes #4304.

To be precise, this was already working: because the deserializer does not append constants to the `deserialized_constants` map until it finishes deserializing them, it is naturally unable to deserialize a recursive subcircuit. This PR just adds the test.

Note that the _serializer_ will still happily attempt (and fail) to serialize CircuitOperations that exhibit infinite recursion. We allow this because `CircuitOperation` is an immutable type; unless you intentionally circumvent the safeguards that prevent mutation (as shown in the test), it is not possible to define a `CircuitOperation` that contains itself.
rht pushed a commit to rht/Cirq that referenced this issue May 1, 2023
Fixes quantumlib#4304.

To be precise, this was already working: because the deserializer does not append constants to the `deserialized_constants` map until it finishes deserializing them, it is naturally unable to deserialize a recursive subcircuit. This PR just adds the test.

Note that the _serializer_ will still happily attempt (and fail) to serialize CircuitOperations that exhibit infinite recursion. We allow this because `CircuitOperation` is an immutable type; unless you intentionally circumvent the safeguards that prevent mutation (as shown in the test), it is not possible to define a `CircuitOperation` that contains itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/serialization area/subcircuits kind/feature-request Describes new functionality triage/accepted A consensus emerged that this bug report, feature request, or other action should be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants