why do we need this? maybe this is why you're still needing to duplicate the get_state() call in complete_atomic_modification()?

before my change prepare_atomic_modification was calling for get_state, I just moved it outside of it in the change of prepare_atomic_modification in the change RGW | fix conditional MultiWrite
if I don't call it in here, we would get a segmentation fault

can you help me understand why manifest would be null here? i would have assumed that _do_write_meta() initializes that with either version of:

-   r = target->get_state(rctx.dpp, &target->state, &target->manifest, false, rctx.y);
-  if (r < 0)
-    return r;
+  target->manifest = manifest;
+  target->state = state;

is there some code path that's calling complete_atomic_modification() but not _do_write_meta()?

yes, it is called also by RGWRados::Object::Delete::delete_obj, but the issue is in the flow of _do_write_meta
somewhere in the code manifest is set to point to nullptr (both of the local manifest and target->manifest)
I still don't know where it is set to point to nullptr

So, I looked at the history of this a bit. Before zipper changed manifests in
88bcacc23ab RGW - Zipper - remove RGWObjectCtx from SAL API
_do_write_meta() explicitly used a different state and manifest (since manifest was inside of state) than the one stored in RGWRados::Object. This was, presumably, the reason for the call to get_state at the start. This means that, by design, complete_atomic_modification was using a different copy of state (and manifest) than _do_write_meta was using. My changes in the above commit preserved that. The "conditional delete fix" commit added a get_state for the state in RGWRados::Object in there, so that check_preconditions could work.

So that's a change from how it was. If we want to use the RGWRados::Object version, we probably should just use that everywhere, rather than having copies at all, but I don't know the issues that might cause.