https://kotlinlang.org logo
Join Slack
Powered by
Hi, it's now been almost six months that we cannot...
# javascript
c
Hi, it's now been almost six months that we cannot use locks on Kotlin/JS because the compiler tries to unlock them twice. Adopting Kotlin/JS was indeed a risk that we decided to take, but it's the first time I ever experienced such a roadblock. For the past three years or so since I started using Kotlin/JS in production, I've never faced any blocking issues. If anyone knows how to get locks to behave normally (even if it's just a workaround), please let me know. If not, I'd appreciate a +1 on the issue so the team will see it. YouTrack issue
r
This version of 
withLock
function seems to work correctly, and (I think) should work exactly the same:
Copy code
private suspend inline fun <T> withLock(block: () -> T): T {
    lock()
    val result = try {
        block()
    } catch (e: Throwable) {
        unlock()
        throw e
    }
    unlock()
    return result
}
c
Wasn't KotlinX.Coroutines available on the Kotlin Playground? I was trying to check your variant, but the reproducer doesn't seem to compile
a
@CLOVIS we are so sorry for your experience. I will take a look on the problem right now and will send you the investigation information as soon as I find something.
Btw, what is the compiler version you use?
c
I originally noticed it on 1.8.20, it was still an issue in 1.8.21.
I originally reported it to the KotlinX.Coroutines team, the YouTrack ticket above was created by them because it wasn't on their side. You can find my full reproducer in the original ticket, but it's buried very deep into the codebase. When they created the YouTrack ticket, Dmitry Khalanskiy added a simplified reproducer that doesn't depend on my code at all, hopefully this is enough information
I will try @Robert Jaros's workaround later today and report back. If you need any more information, don't hesitate to ask (I will reply here faster than on YouTrack, but I do read both)
@Robert Jaros I have tried your workaround, but the issue remains unchanged.
r
Which Kotlin version do you use in your project?
c
1.8.20.
We'll be in 1.9.0 in a few days, hopefully.
We first noticed it 1.8.0 I think
r
I've tested this on 1.8.22 and 1.9.0 with the simplified reproducer code.
c
I see the same result with 1.8.22. Same branch as my previous message.
r
I'll play a bit with pedestal. Perhaps I'll find something new :)
c
To reproduce it using Pedesal:
Copy code
git clone <https://gitlab.com/opensavvy/pedestal.git>
cd pedestal
git switch 101-lock-bug # ← that's where I put your reproducer, you can stay on the 'main' branch if you want to see it for real
./gradlew state:jsNodeTest
Tests 
opensavvy.state.FailureEndToEndTest.usersCannotGetUnsharedCounters
and 
opensavvy.state.FailureEndToEndTest.usersCannotGetACounterThatDoesntExist
will fail because of the bug.
Version numbers are in 
versions.properties
at the root of the repository (we're using #refreshversions)
The 
main
branch is going to change quite a bit very soon in order to use the version catalog, Kotlin 1.9.0 etc. AFAIK, it doesn't impact the problem.
r
Maybe I don't understand, but why have you replaced 
withLock
to 
withLockHack
only in one call?
If I replace all calls in the 
FailureEndToEndTest.kt
the tests pass.
c
Oh wow you're right. Sorry for the time lost.
Thanks a lot, I'll use your workaround in the other places where it happens. Thanks a lot.
Now I'm curious, what did you change exactly? Is it the 
inline
? Is it the 
finally
?
r
finally
I had issues with 
finally
in Kotlin/JS earlier in some complex inline and suspending code. Unfortunately I've never managed to create a reproducer so I couldn't fill an issue.
c
If I get a bit of free time, do you mind if I submit your workaround as a PR directly to kotlinx-coroutines? So it won't affect anyone else until the bug is fixed on the compiler side
r
I've just realized the original 
withLock
is from kotlinx-coroutines :)
c
yes, that's why I originally reported it to them. I think they had a bit of difficulty to provide a reproducer based on the massive project I gave them 😕
r
Please fill free to submit. I hope the compiler will also get a fix because it's definitely a serious compiler bug.
c
I hope so as well, but I've almost been waiting 6 months at this point, so I'll accept any workaround without complaining 😅
If you want me to credit you as co-author of the PR, please send me your GitHub email address (either here or in DM) 🙂
I'll probably do it tomorrow night
Oh, you're the developer of KVision, I didn't recognize you. I guess that explains why you'd spend time on Kotlin/JS haha
r
No need 🙂 You can take all the rewards 😉
Oh yes, I love to workaround Kotlin/JS bugs 😛
K 2
c
Well, we did sign up for it. I much prefer when I find the workaround after a few hours than months though 😅
a
I reproduced it yesterday in 
master
and work on fix, that I hope, I will backport into 
1.9.20
➕ 1
🙏 1
🔝 1
c
@Artem Kobzar do you think it's worth submitting the workaround to Coroutines, then, or will 1.9.20 come before the next KotlinX version anyway?
If you think it's worth it, I can submit it right now
a
I've just prepared the fix for Kotlin master branch, and I think it will be better to make a workaround for kotlinx.coroutines until 1.9.20. So, the problem appears with combination of 
inline
functions and 
finally
block inside of them. The simplest workaround will be to remote 
inline
keyword from the 
withLock
function or to make some wrapper the function that will be a regular function (without 
inline
keyword).
gratitude thank you 1
c
What do you think of the workaround above in this thread? It requires neither removing 
inline
nor creating a wrapper, and the duplication is minimal, so I feel it may be easier to integrate into KotlinX.Coroutines? I doubt removing 
inline
would be accepted, since that's a binary-incompatible change.
gratitude thank you 1
6 Views
Open in Slack
PreviousNext
Powered by