Various dap fixes#1473
Conversation
Thanks for opening this PR! We'll review it soon.I'm a bot that checks for some common problems. I'll update this comment with anything I find. Nothing to report. Nice :) |
| int64_t id = ++m_CurrentID; | ||
| if (id < 1) | ||
| { | ||
| m_CurrentID = 1; | ||
| id = m_CurrentID; | ||
| // if it hasn't already been replaced, replace it with 1 | ||
| std::atomic_compare_exchange_strong(&m_CurrentID, &id, 1); | ||
| id = ++m_CurrentID; |
There was a problem hiding this comment.
the only way this could happen is if the int64 overflowed, right?
in which case, this seems a bit weird to me, e.g.:
- say
m_CurrentID == int64 max - thread A:
int64_t id = ++m_CurrentID; // m_CurrentID == A.id == int64 min - thread A:
if id < 1(true) - thread B:
int64_t id = ++m_CurrentID; // m_CurrentID == B.id = int64 min + 1 - thread A:
std::atomic_compare_exchange_strong(&m_CurrentID, &id, 1); // doesn't happen because A.id != m_CurrentID thanks to thread B - thread A:
id = ++m_CurrentID; // A.id = int64 min + 2 - then a negative number gets returned
There was a problem hiding this comment.
what do you suggest instead?
There was a problem hiding this comment.
With the ID being a 64-bit int I'd honestly just suggest incrementing it and returning it, keeping it atomic ofc. With 32-bit I could see the reason for the bookkeeping here but on a 64-bit integer you could be generating one breakpoint ID per nanosecond and still only ever wraparound after hundreds of years
But the other alternative is a "CAS loop" where each thread keeps attempting to swap in its intended next value until it actually works, which can use the swap-into-expected-parameter behavior of the CAS functions for a marginal speed improvement
Checklist