AtomicCounter starts at 0x100000001
#3000
Labels
Take a Friday Leave a Friday
Stomp the Bugs, Without Much Commitment
Comments
TheBlueMatt
added
the
Take a Friday Leave a Friday
|
Id like to try this issue! I have a question tho, is there a specific reason that the atomic counter in |
|
Not every platform supports atomic u64s. |
TheBlueMatt
added a commit
to TheBlueMatt/rust-lightning
that referenced
this issue
Sep 8, 2024
`AtomicCounter` was slightly race-y on 32-bit platforms because it increments the high `AtomicUsize` independently from the low `AtomicUsize`, leading to a potential race where another thread could observe the low increment but not the high increment and see a value of 0 twice. This isn't a big deal because (a) most platforms are 64-bit these days, (b) 32-bit platforms aren't super likely to have their counter overflow 32 bits anyway, and (c) the two writes are back-to-back so having another thread read during that window is very unlikely. However, we can also optimize the counter somewhat by using the `target_has_atomic = "64"` cfg flag, which we do here, allowing us to use `AtomicU64` even on 32-bit platforms where 64-bit atomics are available. This changes some test behavior slightly, which requires adaptation. Fixes lightningdevkit#3000
TheBlueMatt
added a commit
to TheBlueMatt/rust-lightning
that referenced
this issue
Sep 9, 2024
`AtomicCounter` was slightly race-y on 32-bit platforms because it increments the high `AtomicUsize` independently from the low `AtomicUsize`, leading to a potential race where another thread could observe the low increment but not the high increment and see a value of 0 twice. This isn't a big deal because (a) most platforms are 64-bit these days, (b) 32-bit platforms aren't super likely to have their counter overflow 32 bits anyway, and (c) the two writes are back-to-back so having another thread read during that window is very unlikely. However, we can also optimize the counter somewhat by using the `target_has_atomic = "64"` cfg flag, which we do here, allowing us to use `AtomicU64` even on 32-bit platforms where 64-bit atomics are available. This changes some test behavior slightly, which requires adaptation. Fixes lightningdevkit#3000
TheBlueMatt
added a commit
to TheBlueMatt/rust-lightning
that referenced
this issue
Sep 11, 2024
`AtomicCounter` was slightly race-y on 32-bit platforms because it increments the high `AtomicUsize` independently from the low `AtomicUsize`, leading to a potential race where another thread could observe the low increment but not the high increment and see a value of 0 twice. This isn't a big deal because (a) most platforms are 64-bit these days, (b) 32-bit platforms aren't super likely to have their counter overflow 32 bits anyway, and (c) the two writes are back-to-back so having another thread read during that window is very unlikely. However, we can also optimize the counter somewhat by using the `target_has_atomic = "64"` cfg flag, which we do here, allowing us to use `AtomicU64` even on 32-bit platforms where 64-bit atomics are available. This changes some test behavior slightly, which requires adaptation. Fixes lightningdevkit#3000
TheBlueMatt
added a commit
to TheBlueMatt/rust-lightning
that referenced
this issue
Sep 11, 2024
`AtomicCounter` was slightly race-y on 32-bit platforms because it increments the high `AtomicUsize` independently from the low `AtomicUsize`, leading to a potential race where another thread could observe the low increment but not the high increment and see a value of 0 twice. This isn't a big deal because (a) most platforms are 64-bit these days, (b) 32-bit platforms aren't super likely to have their counter overflow 32 bits anyway, and (c) the two writes are back-to-back so having another thread read during that window is very unlikely. However, we can also optimize the counter somewhat by using the `target_has_atomic = "64"` cfg flag, which we do here, allowing us to use `AtomicU64` even on 32-bit platforms where 64-bit atomics are available. This changes some test behavior slightly, which requires adaptation. Fixes lightningdevkit#3000
TheBlueMatt
added a commit
to TheBlueMatt/rust-lightning
that referenced
this issue
Sep 12, 2024
`AtomicCounter` was slightly race-y on 32-bit platforms because it increments the high `AtomicUsize` independently from the low `AtomicUsize`, leading to a potential race where another thread could observe the low increment but not the high increment and see a value of 0 twice. This isn't a big deal because (a) most platforms are 64-bit these days, (b) 32-bit platforms aren't super likely to have their counter overflow 32 bits anyway, and (c) the two writes are back-to-back so having another thread read during that window is very unlikely. However, we can also optimize the counter somewhat by using the `target_has_atomic = "64"` cfg flag, which we do here, allowing us to use `AtomicU64` even on 32-bit platforms where 64-bit atomics are available. This changes some test behavior slightly, which requires adaptation. Fixes lightningdevkit#3000
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
It should be checking if
low == usize::MAX as u64notlow == 0. Should be harmless cause our uses of it shouldn't care about the value, only that its a fresh one each time, but when I fix it some tests fail, so opening this in the hopes someone else does the work.The text was updated successfully, but these errors were encountered: