Repository navigation
Not thread safe file manipulation results in build failures of multi-module projects #107
Description
Maven Version
3.9.8
Version
0.0.8
Sentry SDK Version
7.14.0
Steps to Reproduce
- Have a multi-module Maven project
- Run
mvn install -T1C(or higher concurrency)
This breaks non-deterministically
Expected Result
Sources are uploaded to Sentry
Actual Result
Getting an error
/bin/sh: 1: /tmp/.sentry-cli10465974399318709029.exe: Text file busy
As far as I understand it is coming from this line
Which is preceded by
After reviewing how thread safe the implementation is, I couldn't pinpoint where the issue is.
Not having other ideas, it seems like the root cause may lie in the JVM bug: https://bugs.openjdk.org/browse/JDK-8068370
It is suggested that a retry could help with that issue. Would be happy to accept a PR that implements a single-attempt retry logic for this bit of code?
Activity
adinauer commentedon Nov 21, 2024
Thanks for opening this issue. We haven't optimized the sentry-maven-plugin for parallel execution (yet).
We could ensure only a single
SentryCliRunneris being used. I'm not certain this is possible as I haven't read up on how parallelism works in Maven. If everything is running inside a single process where the classloader is shared, it shouldn't be a problem to do so.We could then synchronize
runSentryClito only have one invocation ofsentry-clirunning at any time.I'm afraid merely retrying would only fix the problem in a few cases but not all of them and it'd just take longer to fail - not sure that's much better than the current behaviour.
Or do I misunderstand the issue and it's only a problem right after
sentry-cliis copied to a temporary location and unrelated to multiple invocations of it?We're gonna take a look at this in more detail but I can't give an ETA at this point, as we're currently working on v8 of the Java SDK.
nikita2206 commentedon Nov 22, 2024
Thanks for your reply @adinauer
This seems to be happening right after the file is copied, this happens only sometimes. Seems like the exact way to reproduce this is to create a file, mark it executable and then attempt to run it as a subprocess. Given this, it seemed like there's just no way around this bug given the File System features that JVM exposes/doesn't expose.
One other option I'm considering is ignoring the failure in Maven, because being unable to upload sources isn't really a show stopper but only an inconvenience.
nikita2206 commentedon Nov 26, 2024
With the above in mind @adinauer , what do you think about adding the following parameter, which is also part of a few other Maven plugins:
If you think it's acceptable for you guys, I can quickly open a PR.
Otherwise, we are quite happy with how errors looks now in Sentry 🙌
adinauer commentedon Nov 27, 2024
That sounds good, only downside is probably that on a fail you have to upload the sources somehow. I'm not quite sure how that'd play out exactly. On a later upload you just need to make sure you use the same ID that's been packaged with the JAR as
io.sentry.bundle-idsinsentry-debug-meta.properties.We can tackle that problem separately as I'm not sure how exactly we wanna solve it.
A PR would be very welcome! Thanks for the feedback!
adinauer commentedon Nov 27, 2024
Regarding the original problem here, maybe it's sufficient to just sleep for a bit and only then use the freshly copied file.
nikita2206 commentedon Dec 9, 2024
I've realized something basic that I oversaw initially, the reason this was happening is that the plugin tries to source the binary/script from its own JAR, and we can tell it to not do that and provide it with our own
sentry-cli. That was my workaround, so I'm pre-downloading the sentry-cli (currently on a separate build step, but soon will bake it as part of the build image), and configuring the plugin with<sentryCliExecutablePath>${env.SENTRY_CLI_PATH}</sentryCliExecutablePath>to tell it to use that binary.As for the root cause fix, I think you're right and a short sleep might actually help, I'd try that.