Closed Bug 677685 Opened 13 years ago Closed 13 years ago

Implement CompareAndSwapPointers


(Core :: General, defect)

Not set





(Reporter: dzbarsky, Unassigned)



(1 file, 2 obsolete files)

      No description provided.
Blocks: 631527
Attached patch Patch from bug 670967 (obsolete) — Splinter Review
dzbarsky, does this do what you want?
Attachment #551883 - Flags: feedback?
Attachment #551883 - Flags: feedback? → feedback?(dzbarsky)
Attachment #551883 - Attachment is patch: true
Looks good, but try to avoid code duplications with js_CompareAndSwap.
Attachment #551883 - Flags: feedback?(dzbarsky) → feedback+
I'd originally had this in mfbt, but I took it out because I wanted to be able to use mozilla::Mutex.  But in a newer version of the patch (which I'll attach in a moment), I'm no longer using a static mozilla::Mutex because it leaks, and it's hard to instantiate a mutex without allocating any memory (which is necessary for bug 670967).

bsmedberg, What would you think about moving this code into mfbt?
(In reply to David Zbarsky from comment #2)
> Looks good, but try to avoid code duplications with js_CompareAndSwap.

Looking at this more closely, I'm not sure it makes much sense to try to de-duplicate here.  It's a small amount of code, and js_CompareAndSwap's interface is different.

We could still move it to mfbt if we wanted...

David, do you have pointers, or jsword's?  If you have jswords, you'll have to do some nasty casting to make things work with this interface -- the whole point was to avoid this.
Unfortunately they're jswords.  I couldn't get this to work even with the nasty casting.
This apparently doesn't help David after all.  WONTFIX.
No longer blocks: 631527, 670967
Justin, I think your MSVC implementations are wrong.  Shouldn't the argument to InterlockedCompareExchange be in the opposite order?  See
Oh man, and I even had it right in the 64-bit version!

Thanks for noticing, David!
Closed: 13 years ago
Resolution: --- → WONTFIX
Attachment #551883 - Attachment is obsolete: true
Attachment #567010 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.