krisk / fuse-swift

A lightweight fuzzy-search library, with zero dependencies
MIT License
931 stars 110 forks source link

Fix a crashing issue #11

Open chuganzy opened 6 years ago

chuganzy commented 6 years ago

Threading issue. Just fixes crashing issue. I found couple of other threading issues. Will create another PR to address them.

krisk commented 5 years ago

Pardon the tardiness on this response. Have measured the performance of this change? Wondering how locks are doing to impact it.

chuganzy commented 5 years ago

Affects just a little bit - https://github.com/krisk/fuse-swift/blob/master/Example/Tests/Tests.swift#L153 originally it takes 0.040s and after applying it takes 0.042s. However since Swift's array is not thread safe, this change is required to solve a crashing issue.

damienvieira commented 5 years ago

@krisk I was getting the same threading issues, and this did indeed fix the crash. You may consider merge these changes

jdaites commented 4 years ago

Can someone merge this in? I've been getting this crash on my asynchronous calls and It would be awesome to have this in the master branch!

wearhere commented 4 years ago

I didn't see this PR before taking a stab at fixing the crashes myself, in https://github.com/krisk/fuse-swift/pull/41. IMHO that solution is a little bit more idiomatic by using dispatch queues to serialize the writes to these arrays rather than locks, and perhaps that's more performant too, although I haven't tested.

Note that that PR also fixes another problem with these async APIs, which is that they report the wrong indexes.

wearhere commented 4 years ago

@chuganzy curious what the other threading issues you mention at top were. I don't see if you ended up submitting a PR for those.

chuganzy commented 2 years ago

Oops, sorry I completely missed messages on this PR. Since I recently have no time to work on this, it is great if someone else could look take over this if this issue still persists. Regarding the other threading issue, I forgot, but I can help looking into it.