sbt / sbt-pom-reader

Translates xml -> awesome. Maven-ish support for sbt.
Other
76 stars 26 forks source link

NPE in MavenUserSettingsHelper.getUserResolvers #35

Closed mauhiz closed 8 years ago

mauhiz commented 9 years ago

On first try, got this fatal exception :

java.lang.NullPointerException
    at com.typesafe.sbt.pom.MavenUserSettingsHelper$$anonfun$getUserResolvers$2.apply(MavenUserSettingsHelper.scala:36)
    at com.typesafe.sbt.pom.MavenUserSettingsHelper$$anonfun$getUserResolvers$2.apply(MavenUserSettingsHelper.scala:34)
    at scala.collection.TraversableLike$$anonfun$flatMap$1.apply(TraversableLike.scala:251)
    at scala.collection.TraversableLike$$anonfun$flatMap$1.apply(TraversableLike.scala:251)
    at scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
    at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:47)
    at scala.collection.TraversableLike$class.flatMap(TraversableLike.scala:251)
    at scala.collection.AbstractTraversable.flatMap(Traversable.scala:105)
    at com.typesafe.sbt.pom.MavenUserSettingsHelper$.getUserResolvers(MavenUserSettingsHelper.scala:34)

It appears that profile.getRepositories may return null and should be option-wrapped.

metasim commented 9 years ago

@mauhiz Any chance you can offer a simplified pom.xml that triggers the bug?

mauhiz commented 9 years ago

Hi @metasim, I've managed to minimize this issue. It is triggered when settings.xml activates a profile which is not defined:

<?xml version="1.0" encoding="UTF-8"?>
<settings>
<activeProfiles>
    <activeProfile>dev</activeProfile>
</activeProfiles>
</settings>

I believe this is a valid use case (Some projects actually declare this profile to set env variables).

metasim commented 9 years ago

@mauhiz Thanks very much. This helps. I'll look at as soon as I'm able.

jeffstyr commented 8 years ago

Hmm there seems to be something else going on here in addition. I can get this to happen intermittently (before your fix), and in my case my settings.xml doesn't have any active-but-not-defined profiles. (It has two profiles defined, both active.) I added some logging in the case where profiles.get(profileName) returns null, and in fact the value is present in the map! Since it's intermittent (it usually happens, but not always, and it's not always the same profile that appears to be missing), this points me to a concurrency issue--is it possible that the MavenSetting are getting read in a separate thread, without enough coordination? I'm fuzzy on how sbt evaluates settings, but it does appear to use multiple threads. Your filtering fixes it (and is necessary to handle the test case above) but if there's a concurrency problem then resolvers could end up going intermittently missing. I haven't tried to whittle it down to a minimal test case yet, but I wanted to mention what I'm seeing in case you have any ideas. (I have a project with a large number of sub-modules, and that's probably necessary in order to trigger it. I'll try to see if I can get it to happen in a standalone test.)

metasim commented 8 years ago

@jeffstyr Is the backtrace the same as the originally reported one?

I'd also be curious to know what happens if you create a combination of the multiple submodules test with the can-handle-missing-profile test.

jeffstyr commented 8 years ago

@metasim Yep exact same backtrace. I think it's sort of a coincidence--two separate issues (actually missing profile v. spuriously missing). I did some more experimentation and I've convinced myself that the issue I'm hitting is that the Maven data structures aren't threadsafe--in this case, specifically org.apache.maven.settings.Settings. Although effectiveSettings is evaluated under appropriate synchronization by sbt, the actual returned Settings data structure internally mutates when its accessor methods are called (it's kind of perniciously un-thread-safe), so using it from multiple threads is problematic, and sbt is aggressively threaded about evaluating settings. I can reproduce it in a standalone case with a large number of subprojects (e.g., 200)--it doesn't happen on every run, but more than 50%.

The only safe fix I can think of would be to populate an immutable data structure with the needed data from the Settings and change the effectiveSettings key to evaluate to that. Looking at the implementation of org.apache.maven.model.Model, I suspect there could be problems with effectivePom as well, but I haven't run into them yet.

(As I mentioned, your fix will prevent the NPE here, but the problem should then manifest as some submodules missing some resolvers. I'm trying to figure out of I can write a test which generates the submodules dynamically, rather than checking in 200 tiny POMs.)