Created attachment 14152 [details]
Thread dump

Hmm, looks like this may in fact happen more generally, during module
enablement change. Needs to be fixed somehow. Using static
synchronization on URLMapper is probably the real evil here -
allInstances may block on another thread (alas), which may in turn
need URLMapper. So you should have only "local" synchronization to
protect the integrity of the cache - i.e. do not hold any visible
locks while calling into Lookup.

Created attachment 14154 [details]
Another deadlock encountered in a test

Looks like all module/instance-related unit tests deadlock as a result
of this. Cannot run core tests.

Hmmm, I did not try core tests before commit.

I'm solving stupid problem how to dump threads on Windows for
deadlocked test which runs in separate process forked by xtest. I do
not have kill utility on Windows and Cygwin does not work either.

As for the fix I see two options. #1) revert my change so that Lookup
is initialized staticaly and cache only instances or #2) restrict
URLMapper registration to META-INF/services.

In second case the URLMapper would call only
Lookups.metaInfServices(...) and that should prevent these deadlocks.
However what ClassLoader I should pass to metaInfServices method,
without calling Lookup.lookup(ClassLoader)? And it is sort of
incompatible change.

Suggest third option: never call L.R.allInstances from within the body
of getInstances. Initialize cache to be the empty list, update it only
from within resultChanged, and in the static initializer try to
initialize it asynch (ugh).

Or trickier but maybe more effective - call L.R.allInstances synch but
do not do so while holding the monitor on URLMapper.class. So
getInstances might look like e.g.

private static List/*<URLMapper>*/ cache = new ArrayList();
private static List getInstances() {
    boolean doinit;
    synchronized (cache) {
        doinit = result == null;
    }
    if (doinit) {
        // If really necessary, replan this block to RP:
        result = Lookup.getDefault().lookup(new
Lookup.Template(URLMapper.class));
        updateInstances();
        result.addLookupListener(new LookupListener() {
            public void resultChanged(LookupEvent ev) {
                updateInstances();
            }
        });
    }
    synchronized (cache) {
        return new ArrayList(cache);
    }
}
private static void updateInstances() {
    List resultInstances = result.allInstances();
    synchronized (cache) {
        cache.clear();
        cache.addAll(resultInstances);
    }
}

Fixed in:
Checking in src/org/openide/filesystems/URLMapper.java
new revision: 1.27; previous revision: 1.26

Test is passing now.

closing