Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-threaded access not allowed when define Java.type and javascript function in karate-config.js on karate 1.3.0+ #2204

Closed
ericdriggs opened this issue Dec 5, 2022 · 5 comments
Assignees

Comments

@ericdriggs
Copy link

ericdriggs commented Dec 5, 2022

Minimal viable example:
https://github.com/ericdriggs/karate-multi-threaded-access-bug

Versions effected: 1.3.0, 1.4.0.RC1

Versions not effected: 1.2.0

Steps to reproduce:

  • karate 1.3.0
  • java 11
  • define Java.type and java functions in karate-config.js using karate.callSingle
  • multi-threaded access to javascript function

Mitigations:

  • use karate 1.2.0 or 1.4.0.RC1
  • use karate.call instead of karate.callOnce

Test machine:

  • Mac m1
@ericdriggs ericdriggs changed the title Multi-threaded access not allowed when define Java.type and javascript function in karate-config.js Multi-threaded access not allowed when define Java.type and javascript function in karate-config.js on 1.3.0 Dec 5, 2022
@ericdriggs ericdriggs changed the title Multi-threaded access not allowed when define Java.type and javascript function in karate-config.js on 1.3.0 Multi-threaded access not allowed when define Java.type and javascript function in karate-config.js on karate 1.3.0+ Dec 5, 2022
@ptrthomas ptrthomas self-assigned this Dec 6, 2022
@ptrthomas ptrthomas added the bug label Dec 6, 2022
@ptrthomas
Copy link
Member

@ericdriggs thanks ! was able to replicate this in our local test suite, refer attached commit

we now synchronize in one more place to make sure the graal function evaluation for config-js is single-threaded

@ptrthomas ptrthomas added the fixed label Dec 6, 2022
@ptrthomas ptrthomas added this to the 1.4.0 milestone Dec 6, 2022
@ptrthomas
Copy link
Member

1.3.1 released

ptrthomas added a commit that referenced this issue Jan 31, 2023
and #2222 could be a consequence of that
so we are making sure that any js functions in a callSingle result
are re-hydrated to current context
@ptrthomas
Copy link
Member

@ericdriggs turns out the extra "global lock" we added to fix this may have been overkill and would lock every evaluation of karate-config.js. and we got a report in #2222 which I suspect is a consequence

so I have just removed the lock. instead, we post process any JS functions that a karate.callSingle() may attempt to return. I think this is a reasonable compromise. more details in this commit: 8cd5a78

would be great if you can check if your use of karate.callSingle() is ok

@ericdriggs
Copy link
Author

ericdriggs commented Feb 3, 2023

@ptrthomas
Yeah I've noticed that when I tried bumping to 1.4 the tests took 2x as long, so we're still on 1.2 for now.
Glad to hear you removed the lock -- multi-threaded access to a shared context in graal is certainly a pain.
Don't fully grok changes you linked -- in general seems that some sort of copying to prevent shared access without mutexes needed. Hope that's the change you've made.

Is there a new RC to test?

@ptrthomas
Copy link
Member

@ericdriggs yes you are right. I will release an RC next week

ptrthomas added a commit that referenced this issue Feb 4, 2023
ptrthomas added a commit that referenced this issue Feb 4, 2023
This reverts commit 94d7afe.

ci failed although it worked locally, so abort this attempt
ptrthomas added a commit that referenced this issue Feb 4, 2023
ptrthomas added a commit that referenced this issue Feb 4, 2023
that was very stable for a while #2222 #2204
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants