inout paremeter and thread safety

I've run in a hard to track craskh. After some investigation, I think it is related to the use of inout parameter in a thread.


I'll try to explain:


class myClass : NSObject {

     var aVar : aStruct

     func myFunc(param: inout aStruct) {
     }

     func calling() {
          myFunc(param: &aVar)
     }
}


When I call this func from another thread, I get random crashes (BAD_ACCESS).


I understand it is not safe to pass address in a thread.

So I try to change like this:


     func calling() {
          var localVar = aVar
          myFunc(param: &localVar)
          aVar = localVar
     }


That seems to solve the crash, but is that a correct implementation ?


So, is iy thread safe to call

myFunc(param: &localVar)

if localVar is declared in the function ?


This is a serious limit to the use of inout (be careful if the func is to be called from another thread) and I find it really poorly documented (I've found no reference to this in Swift language). Is it detailed somewhere ?


Edited : Note. I feel the crash are much more frequent after I switched to Swit 3 and 4. Is there any reason for that or just an impression ?

Answered by QuinceyMorris in 277323022

The thing about return values was part of Quinn's general advice, and doesn't help much with your case. In general, if a value is being returned in an inout parameter (a use-case in you'll find in the Cocoa APIs, for example, though not very commonly), and if there's a chance that the inout references are being passed across thread boundaries, then returning multiple values, as a direct function result instead, would limit the possibility of thread-unsafety regarding that returned value. However, if the returned value is then assigned to a mutable variable that itself is accessed by multiple threads (your case, sort of), then that is a second cause of unsafety, and the strategy doesn't help.


In your case, the larger cause of the problem is that you're calling a function (the "calling ()" function), which has global side-effects. The best way to solve the problem is to eliminate the side-effects. IOW, delete the "aVar" property. Obviously, we can't tell if there's an alternate approach that would let you do that, because your sample code is too minimal.


A second approach would be to ensure that the function is called on a single, pre-determined thread, such as the main thread, or equivalently the main queue, or a serial queue. That means your multi-threaded code would have to hop between threads, which could be a pretty big change to the code.


Or, you can simply use a lock to protect access the othe "aVar" property. This doesn't obviously require any serious code redesign, except you have to ensure that there are no potential deadlock.

That seems to solve the crash, but is that a correct implementation ?


So, is iy thread safe to call

myFunc(param: &localVar)

if localVar is declared in the function ?


Depends on many things, your `aStruct`, `myFunc`, or how you schedule your threads, or many other things which I cannot think of now.


For example, if `aStuct` is bigger than a size where CPU can move it atomically, copying the struct cannot be thread-safe.

Think what happens when `aVar = localVar` and `localVar = aVar` executed simultaneously.


If you really want to make your code thread-safe, you may need to use some sort of synchronization.

I think it’s important to distinguish between what works on any given implementation and what is defined to work. OOPer’s comments about atomicity, for example, are about implementation, not definition. Let me illustrate this with an example from C (and the reason I’m choosing C, not Swift here, is something I’ll make clear below):

  1. Imagine a memory block like this:

    static int * b1 = calloc(1, 1024);

    .

  2. Now consider code like this:

    *b1 += 1

    being called from two different threads.

On any given CPU architecture it is very likely that this will produce a result of either 0, 1 or 2, that is, the increments will happen or they won’t. However, according to the C memory model this is undefined and the system (the C compiler and the runtime underneath) is allowed to do whatever it wants in this case. For example:

  • It could produce 3, or -1.

  • It could detect the race condition at compile time and fail to compile.

  • It could detect the race condition at runtime and trap.

  • It could exploit the fact that this is condition is ‘impossible’ to optimise away the code entirely.

If you’re not already familiar with this sort of thing, you should read the class What Every C Programmer Should Know About Undefined Behavior, part 1, part 2 and part 3.

The problem with talking about concurrency in Swift is that it doesn’t have a well-defined memory model. You can see the beginnings of this start to come together in the Ownership Manifesto, but that’s some way from being done. Thus, it’s very hard to separate implementation from definition because there’s no definition )-:

Note The first step along this path has already been taken, namely SE-0176 Enforce Exclusive Access to Memory. The Concurrency section of that doc will be of interest to you.

The ad hoc guidelines folks typically follow here are:

  • Incrementing and decrementing reference counts is assumed to be thread safe.

  • Everything else can be either:

    • Shared between threads if it’s immutable
    • Must be protected from multi-threaded access if it’s mutable

Note The immutable case above is tricky because it’s possible for something to look immutable from the outside but actually be mutable internally. A good example of this is copy-on-write data structures. If you build such a data structure it’s a good idea to design it to be thread safe in this case.

Coming back to your specific problem, it’s hard to offer advice without knowing more about what you’re doing. Specifically, is

aVar
being access be multiple threads simultaneously? If so, this is definitely not safe and you may well crash if
aStruct
is complex. Your local variable workaround might reduce the incidences of these crashes but it’s still not correct because you still have multiple threads accessing
aVar
.

My general advice for this sort of thing:

  • Make as much stuff as you can immutable.

  • For any given mutable value, define a serialisation context (a thread, a serial queue, a mutex) that’s responsible for that value, and only access the value within that context.

  • Take advantage of function results; I rarely use

    inout
    parameters because Swift let’s me return multiple items from a function. Function results are naturally serialised because no one can get at the result until the function is done.
  • Function results make it easier to pass data between contexts. So you can write code like this:

    let value = someFunction(…)
    queue.async {
        mutableValue = value
    }

    So all the complex code is within

    someFunction(…)
    and, when it’s done, it return the result to the caller when the applies the value from within a serialisation context (in this example I’m assuming a dispatch serial queue).

Share and Enjoy

Quinn “The Eskimo!”
Apple Developer Relations, Developer Technical Support, Core OS/Hardware

let myEmail = "eskimo" + "1" + "@apple.com"

Thanks Quinn and OOPer for the explanations, but I still have question.


In fact, myStruct is pretty complex (several multi dimensional arrays inside) and thus it cannot be treated atomically. In fact, crashes seemed to occur when accessing index of dimensional array.


Here is what I read that made me think of the pattern with a local var to pass in inout :

stackoverflow.com/questions/39569114/swift-3-0-error-escaping-closures-can-only-capture-inout-parameters-explicitly


Because the closure that is passed to

dispatch_async
escapes the lifetime of the function
foo
, any changes it makes to
val
aren't written back to the caller's
str
– the change is only observable from being passed into the completion function.

In Swift 3,

inout
parameters are no longer allowed to be captured by
@escaping
closures, which eliminates the confusion of expecting a pass-by-reference. Instead you have to capture the parameter by copying it, by adding it to the closure's


The pattern with local var seems to have eliminated crash (I could not get any new crash whatever I tried).

But I would like to make sure it is effectively correcting the problem ; from OOPer reply, I understand not.

>> I would like to make sure it is effectively correcting the problem


No, it's not. It just makes the crash happen less often. In a case like that, that's making the bug worse, not better. (If it's rare, it will seem like a random inexplicable bug, so be harder to fix in the future.)


>> escaping-closures-can-only-capture-inout-parameters-explicitly


The code structure that exhibits your problem doesn't have any closures.

I agree, if bug is just less frequent it is just worse. That's why I did ask the question to begin with.


So, reading from all, I would need to check if the following would be a robust solution :

" if `aStuct` is bigger than a size where CPU can move it atomically, copying the struct cannot be thread-safe"

-> keep inout when var size is small (eg Bool)

Otherwise

'Take advantage of function results"

-> that's reduce significantly the opportunity to use inout ?


I read the various Swift evolution about inout ; in SE-0035, at the end :

A possible extension of this proposal is to introduce a new capture kind to ask for shadow copy capture:

In discussion, we deemed this rare enough not to be worth the added complexity. An explicit copy using a new

var
declaration is much clearer and doesn't require new language support.

But this is about closures. Isn't capture for thread a somehow similar situation ?

Anyway, thanks to all for your help.

I don't think your summary is safe, either. The size of the struct is not a good test whether copying is atomic. Your problem is (as usual in such scenarios) not how to get atomicity, but how to get thread safety. Whatever portions of your code access a shared mutable data item (i.e. "aVar") need synchronization to provide thread safety.


The use of return values doesn't help with thread safety (at least, not automatically, though it might as part of a different strategy), it helps to avoid "inout" parameters, which helps to avoid subtle behaviors related to "inout" parameters. However, "inout" was never your problem. Your problem was always that the code isn't thread safe.

I'm not really clear on what I should do to make it thread safe.


I've read :


For any given mutable value, define a serialisation context (a thread, a serial queue, a mutex) that’s responsible for that value, and only access the value within that context.

=> How do I achieve this ?

Take advantage of function results; I rarely use

inout
parameters because Swift let’s me return multiple items from a function. Function results are naturally serialised because no one can get at the result until the function is done.

=> So isn't it intrinsically thread safe ?

Accepted Answer

The thing about return values was part of Quinn's general advice, and doesn't help much with your case. In general, if a value is being returned in an inout parameter (a use-case in you'll find in the Cocoa APIs, for example, though not very commonly), and if there's a chance that the inout references are being passed across thread boundaries, then returning multiple values, as a direct function result instead, would limit the possibility of thread-unsafety regarding that returned value. However, if the returned value is then assigned to a mutable variable that itself is accessed by multiple threads (your case, sort of), then that is a second cause of unsafety, and the strategy doesn't help.


In your case, the larger cause of the problem is that you're calling a function (the "calling ()" function), which has global side-effects. The best way to solve the problem is to eliminate the side-effects. IOW, delete the "aVar" property. Obviously, we can't tell if there's an alternate approach that would let you do that, because your sample code is too minimal.


A second approach would be to ensure that the function is called on a single, pre-determined thread, such as the main thread, or equivalently the main queue, or a serial queue. That means your multi-threaded code would have to hop between threads, which could be a pretty big change to the code.


Or, you can simply use a lock to protect access the othe "aVar" property. This doesn't obviously require any serious code redesign, except you have to ensure that there are no potential deadlock.

I'll try the third strategy. Excuse my ignorance, how do I put a lock on the aVar ?


I've used thread Sanitizer. Wonderful, it spotted immediately where I had a problem, with a very easy correction: getting call to the conflicting memory out of the Dispatch_Queue.


Wonderful tool !

how do I put a lock on the aVar ?

There’s no automatic support for this. Instead you have to allocate your lock next to the data you want to protect:

class myClass : NSObject {  

    var aVar : aStruct  
    let l = NSLock()

    …
}

And then guard all accesses with code like this:

l.lock()
… access stuff …
l.unlock()

IMPORTANT This pattern is inherently risky because it’s easy to forget to unlock. I typically avoid this problem with an extension on

NSLock
. I’ve pasted that code in at the end of this post but, IMO, it’s just a distraction because you have bigger problems )-:

However, I suspect that won’t work in your case because of the nature of

aVar
. You wrote:

myStruct is pretty complex (several multi dimensional arrays inside)

What does your access pattern on those arrays look like? Most folks who ask questions that involve large multi-dimensional arrays and threading and doing some sort of complex number crunching, and thus need actual parallelism. If you put a big lock around the data, adding more threads won’t help because they’ll all get stuck on the lock.

The solution is very much dependent on the type of number crunching you’re doing. For example:

  • If you have frequent read accesses and infrequent write accesses (imagine, say a dot product), you could make the input immutable and then use a lock to protect the output.

  • If you only need to access a small ‘area’ of the array, you could divide it up into tiles and have each thread work on a tile.

If you can describe the data you’re dealing with and the access patterns of your algorithm, we should be able to make more concrete suggestions.

Share and Enjoy

Quinn “The Eskimo!”
Apple Developer Relations, Developer Technical Support, Core OS/Hardware

let myEmail = "eskimo" + "1" + "@apple.com"

Here’s the extension I discussed above:

extension NSLock {
    func withLocked<ReturnValue>(_ body: () throws -> ReturnValue) rethrows -> ReturnValue {
        self.lock()
        defer {
            self.unlock()
        }
        return try body()
    }
}

And here’s you might use it:

l.withLocked {
    // … access stuff …
}
inout paremeter and thread safety
 
 
Q