scwhittle commented on code in PR #29742:
URL: https://github.com/apache/beam/pull/29742#discussion_r1425213558
##########
sdks/python/apache_beam/runners/worker/sdk_worker.py:
##########
@@ -491,7 +492,10 @@ def get(self, instruction_id, bundle_descriptor_id):
# Make sure we instantiate the processor while not holding the lock.
processor = bundle_processor.BundleProcessor(
self.runner_capabilities,
- self.fns[bundle_descriptor_id],
+ # Reduce risks of concurrent modifications of the same protos
+ # captured in bundle descriptor when the same bundle desciptor is used
+ # in different instructions.
+ copy.deepcopy(self.fns[bundle_descriptor_id]),
Review Comment:
Not sure if it would be safer to use Message generated CopyFrom method
instead. Unclear to me if deepcopy would do something odd with lower-level upb
impl.
deepcopy isn't mentioned here
https://protobuf.dev/reference/python/python-generated/
##########
sdks/python/apache_beam/runners/worker/sdk_worker.py:
##########
@@ -491,7 +492,10 @@ def get(self, instruction_id, bundle_descriptor_id):
# Make sure we instantiate the processor while not holding the lock.
processor = bundle_processor.BundleProcessor(
self.runner_capabilities,
- self.fns[bundle_descriptor_id],
+ # Reduce risks of concurrent modifications of the same protos
+ # captured in bundle descriptor when the same bundle desciptor is used
+ # in different instructions.
+ copy.deepcopy(self.fns[bundle_descriptor_id]),
self.state_handler_factory.create_state_handler(
self.fns[bundle_descriptor_id].state_api_service_descriptor),
Review Comment:
use the deepcopied descriptor_id here too?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
