Skip to content

Commit

Permalink
Fix concurrency bug in ConcurrentSlab.replace. (FuelLabs#2378)
Browse files Browse the repository at this point in the history
So what happens here is that we get the following panic during
`ConcurrentSlab.replace` execution:

```
thread 'main' panicked at 'rwlock read lock would result in deadlock',
/rustc/ddcbba036aee08f0709f98a92a342a278eae5c05/library/std/src/sys/unix/locks/pthread_rwlock.rs:72:13
```

This happens due to `operator !=`, we call `PartialEq` and eventually
end up calling `ConcurrentSlab.get`, which fails due to existing
read/write lock already on the stack.

The fix is to make sure to get a read lock only, do the comparison,
and then get a read/write lock just for the write.

Closes FuelLabs#2341.
  • Loading branch information
tritao authored Jul 27, 2022
1 parent 10f59dd commit 82238bc
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 4 deletions.
17 changes: 13 additions & 4 deletions sway-core/src/concurrent_slab.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,20 @@ where
}

pub fn replace(&self, index: TypeId, prev_value: &T, new_value: T) -> Option<T> {
let mut inner = self.inner.write().unwrap();
let actual_prev_value = &inner[*index];
if actual_prev_value != prev_value {
return Some(actual_prev_value.clone());
// The comparison below ends up calling functions in the slab, which
// can lead to deadlocks if we used a single read/write lock.
// So we split the operation: we do the read only operations with
// a single scoped read lock below, and only after the scope do
// we get a write lock for writing into the slab.
{
let inner = self.inner.read().unwrap();
let actual_prev_value = &inner[*index];
if actual_prev_value != prev_value {
return Some(actual_prev_value.clone());
}
}

let mut inner = self.inner.write().unwrap();
inner[*index] = new_value;
None
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[[package]]
name = 'contract_caller_as_ret'
source = 'root'
dependencies = [
'core',
'std',
]

[[package]]
name = 'core'
source = 'path+from-root-277F468596DF2097'
dependencies = []

[[package]]
name = 'std'
source = 'path+from-root-277F468596DF2097'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
license = "Apache-2.0"
name = "contract_caller_as_ret"
entry = "main.sw"

[dependencies]
core = { path = "../../../../../../../sway-lib-core" }
std = { path = "../../../../../../../sway-lib-std" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[
{
"inputs": [],
"name": "test_function",
"outputs": [
{
"components": null,
"name": "",
"type": "bool",
"typeArguments": null
}
],
"type": "function"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
contract;
use std::contract_id::ContractId;

abi MyContract {
fn test_function() -> bool;
}

impl MyContract for Contract {
fn test_function() -> bool {
true
}
}

fn caller(address: ContractId) -> ContractCaller<_> {
abi(MyContract, address.value)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
category = "compile"
validate_abi = true

0 comments on commit 82238bc

Please sign in to comment.