[contacts/ContactModifyJob] Avoid race condition
ClosedPublic

Authored by poboiko on Mar 21 2020, 11:16 AM.

Details

Summary

This patch is similar to D28178: [contacts] Fix ContactCreateJob with a photo. Although uid is populated, and we can
freely dispatch both contact and photo modify requests in parallel, the
following race condition still sometimes happens:

  1. Contact modify request dispatched
  2. Contact photo modify request dispatched
  3. Photo reply arrives

At this point, d->lastContact is not yet populated, and we call
processNextContact(), which repeats steps 1 and 2 for the very same contact.
Which could happen again and again, if photo reply arrives first again.

Instead, I suggest to fire photo modify request only after we receive reply
for contact modify request, ensuring everything happens in the following order:

  1. Contact modify request dispatched
  2. Contact modify reply received
  3. Contact photo modify request dispatched
  4. Contact photo reply received
Test Plan
  1. Modify a contact via KAddressBook
  2. Check logs
  3. (without patch) Bunch of modify requests somtimes gets fired
  4. (with patch) Only one modify request gets fired

Diff Detail

Repository
R477 KGAPI Library
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
poboiko created this revision.Mar 21 2020, 11:16 AM
Restricted Application added a project: KDE PIM. · View Herald TranscriptMar 21 2020, 11:16 AM
poboiko requested review of this revision.Mar 21 2020, 11:16 AM
poboiko edited the test plan for this revision. (Show Details)Mar 21 2020, 11:17 AM
dvratil requested changes to this revision.Mar 21 2020, 12:21 PM

Looks good, thanks for the fix. Just move the code to a separate function, please :)

src/contacts/contactmodifyjob.cpp
170–181

Please move the code to a new function (e.g. Private::updatePhoto(const ContactPtr &contact))

This revision now requires changes to proceed.Mar 21 2020, 12:21 PM
poboiko updated this revision to Diff 78162.Mar 21 2020, 1:33 PM

Create a dedicated function for photo uploading

dvratil accepted this revision.Mar 21 2020, 2:17 PM

Thanks!

This revision is now accepted and ready to land.Mar 21 2020, 2:17 PM
This revision was automatically updated to reflect the committed changes.