Raynes/fs

cwd not thread-safe

Closed this issue · 6 comments

franks42 commented

when you use fs.core/chdir from different threads concurrently, the single instance of cwd will get updated by all, which makes it kind of indeterministic,,, an alternative would be to make cwd thread-local or maybe inheritable-thread-local is better, but unfortunately that would break backwards compatibility as you would need a double deref "@@cwd" - or maybe there are other solutions (?).
I've forked your fs and implemented a solution based on inheritable-thread-local - didn't want to add a pull request as it breaks backwards compatibility - i'd be happy to contribute it - pls take a look: https://github.com/franks42/fs
(the inheritable-thread-local is based on useful.utils/thread-local but initializes the thread-local value with the parent's thread one).
-Regards, FrankS.

Raynes commented

Hey. Thanks for bringing this up. Just wanted to let you know that I've seen this issue. I'll take a look within the next week or so. I've got a whole lot going on at the moment.

Just a heads up so that you don't think I'm ignoring you if I don't respond for a couple of days.

franks42 commented

no hurries - I won't feel ignored - best wishes for your mom - fs.

Raynes commented

Hey. Sorry I haven't looked at this in so long.

@amalloy Would you want this in useful? https://github.com/franks42/fs/blob/master/src/fs/utils.clj#L31

franks42 commented

Thanks for picking this up again.

Did you think about how to make fs more thread-safe ?

There may be alternative ways to go about it besides my suggestion...

Thanks, FrankS.

On Apr 10, 2012, at 10:04 AM, Anthony Grimesreply@reply.github.com wrote:

Hey. Sorry I haven't looked at this in so long.

@amalloy Would you want this in useful? https://github.com/franks42/fs/blob/master/src/fs/utils.clj#L31

Reply to this email directly or view it on GitHub:
#22 (comment)

pbiggar commented

We've been using the old version of fs (0.9 and 0.11.1) until now, and it looks like the changes to *cwd*, and the removal of with-cwd break things for us. Can we go back to the old way, or was there something broken about it?

Raynes commented

I decided to just make cwd a dynamic var. You can still simulate mutation by wrapping your code in with-mutable-cwd and using set! (or the new chdir function). I think this is the simplest and most satisfactory solution. Less magic.

@pbiggar The latest changes might make things easier on you.