Developer's Guide needs correction on garbage collection

• Key: DERBY-3612
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Kim Haase
• Reporter: Kim Haase
• Component/s: Documentation
• Affects Version/s: 10.3.2.1
• Fix Version/s: 10.4.2.0, 10.5.1.1

Description

In a comment on DERBY-3585, Martin Zaun pointed out the following error:

devguide/tdevdvlp20349.dita
   - found a flatly wrong statement but did NOT correct here since
     unrelated to server shutdown authentication:
     "You cannot explicitly request that the JVM unload a class, but you
     can ensure that the EmbeddedDriver class is unloaded by using a
     System.gc() to force it to garbage collect classes that are no longer
     needed. Running with -nogc or -noclassgc definitely prevents the class
     from being unloaded and makes you unable to restart Derby in the same
     JVM."

     System.gc() is only a suggestion to the Runtime to garbage-collect, it
     cannot be enforced, and there's no guarantee whatsoever that GC has
     run and any classes been unloaded. Likewise it's most probably not
     guarantueed that -nogc or -noclassgc definitely (!) prevent a class
     from being unloaded (a JVM may ignore these options...)

This needs to be fixed. Thanks, Martin.

One more question: are -nogc and -noclassgc java command line options, or options to some other command? I can't find them in the java command documentation -- http://java.sun.com/javase/6/docs/technotes/tools/windows/java.html. There is an -Xnoclassgc option, however.

Comments

[Kim Haase]

Attaching DERBY-3612.diff and tdevdvlp20349.html. This patch

 - corrects the description of System.gc() to make clear that it requests but does not force garbage collection

 - removes the references to the -nogc and -noclassgc options, since these appear to have been obsolete since Java 2 came in, and replaces them with the current non-standard -Xnoclassgc option.

 - tidies up the language a bit (changes passive voice to active, etc.)

I'd be grateful if someone could check the correctness of the modified info. If I don't hear in a week or so, though, I'll go ahead and commit this.

[Martin Zaun]

Hi Kim,

while your changes on the garbage collection invocation and options are all correct and look good to me, I've developed doubts that the described underlying problem really is one: that the class EmbeddedDriver needs to unloaded or Derby can't be restarted in the same VM again.

To be sure, I've seen such static initialization issues before, and in the appserver we treated them as a bug and fixed them (if I remember correctly).

But I'm not sure if Derby indeed has this problem -- at least, I was unable to reproduce it.

Here's what I tried:
1) run ij with JVM options -Xnoclassgc (and -verbosegc)
2) connect 'jdbc:derby:testdb';
3) select * from test;
4) connect 'jdbc:derby:;shutdown=true';
5) repeat steps 2-4 x times

No errors except the expected "ERROR XJ015: Derby system shutdown." message. Also tried the network case with -Xnoclassgc on the client and server side to be sure; again no errors.

I also tried with a different JDBC client by basically modifying the Derby demo's SimpleApp to load the driver and shutting down the engine multiple times:

    public static void main(String[] args)
    {
        new SimpleApp().go(args);
        new SimpleApp().go(args);
        new SimpleApp().go(args);
        System.out.println("SimpleApp finished");
    }

Again, no problems.

All these tests makes me doubt the following statement in the documentation:

>>> To restart Derby successfully, the JVM needs to unload org.apache.derby.jdbc.EmbeddedDriver, so that it can reload it when it restarts Derby. (Loading the local driver starts Derby.) <<<

and definitely proves wrong another statement:

>>> Running with the non-standard option -Xnoclassgc definitely prevents the class from being unloaded and makes you unable to restart Derby in the same JVM. <<<

It would be good if someone who's been much longer involved with Derby than I have a look at this:

- Was there, perhaps, a class initialization issue in the past that's been fixed? If so, the entire two paragraphs in the documentation should be deleted.

- Or was the issue of a different kind than my test tried to reproduce? If so, some more information on the problematic use case in the documentation would be helpful, since this is what the user needs to avoid; my simple test case above, which fits the current description, turns out not to be problematic, even with -Xnoclassgc.

Martin

[Kim Haase]

Thanks, Martin. I hope we can get an answer to this question. I tried removing a System.gc() call from one of my programs for DERBY-3200, which starts Derby in embedded mode, stops it, restarts it, then stops it again. I removed the System.gc() call from between the stop and restart. There seems to be no change in behavior. But this is a fairly trivial example.

[Kim Haase]

Another issue I forgot to mark "patch available". This time I really will commit the patch if I don't hear anything in a few days.

[Knut Anders Hatlen]

Kim, the patch looks like an improvement to me, so +1 to commit.

I don't have any definite answers to your questions, but I have some
more background information:

Classes are not unloaded until their class loader is eligible for
garbage collection[1]. If the driver class was loaded in the system
class loader, it'll probably stay alive until the JVM is terminated.

Since Class.forName("...EmbeddedDriver") only performs work if the
EmbeddedDriver class is not loaded, many of the examples in the
documentation additionally use a call to newInstance() to ensure that
the initialization is performed in the cases where the driver has been
loaded previously in the same JVM instance, the engine has been shut
down, but the driver class has not been unloaded. See also this[2]
discussion and DERBY-503, which was logged in order to make the
documentation state explicitly that newInstance() is recommended when
(re)loading the driver.

When Martin tested the shutdown behaviour, he used the ij tool and the
SimpleApp demo. Both of these loaded the driver with Class.forName() +
newInstance(), so it didn't matter whether or not the driver class was
unloaded. I wrote a simple test myself which only executed the
following code twice in the same JVM:

        Class.forName("org.apache.derby.jdbc.EmbeddedDriver");
        try {
            DriverManager.getConnection("jdbc:derby:;shutdown=true");
        } catch (SQLException e) {
            // XJ015 is the expected system shutdown exception
            if (!"XJ015".equals(e.getSQLState())) {
                throw e;
            }
        }

Note that I didn't call newInstance().

The second time it was executed, it threw this exception:

Exception in thread "main" java.sql.SQLException: org.apache.derby.jdbc.EmbeddedDriver is not registered with the JDBC driver manager
        at org.apache.derby.jdbc.AutoloadedDriver.getDriverModule(Unknown Source)
        at org.apache.derby.jdbc.AutoloadedDriver.connect(Unknown Source)
        at java.sql.DriverManager.getConnection(DriverManager.java:582)
        at java.sql.DriverManager.getConnection(DriverManager.java:207)
        at ReloadDriver.main(ReloadDriver.java:15)

If the code used Class.forName(...).newInstance() the second time it
loaded the class, it didn't throw an exception. Calling System.gc()
between the shutdown and the reloading wouldn't make any difference,
since the driver class wasn't eligible for garbage collection.

In fact, I think mentioning garbage collection of classes when
documenting engine shutdown is more confusing than enlightening,
especially given that (a) classes are not collected until their class
loader is, and (b) System.gc() doesn't give any guarantees. Since the
only robust solution is to call newInstance() when reloading the
driver, I think it's better to document that and not mention the
garbage collector at all.

[1] http://java.sun.com/docs/books/jls/third_edition/html/execution.html#12.7
[2] http://www.nabble.com/derby-jdbc-load-driver-to577168.html

[Kim Haase]

Thanks very much, Knut Anders. I have committed the patch, but I've also assigned myself to DERBY-503 so I can make the further corrections you suggest -- dropping the mention of garbage collection entirely and recommending a call to newInstance instead.

Committed DERBY-3612.diff to documentation trunk at revision 666226.

[Kim Haase]

Reopening to backport to 10.4.

[Kim Haase]

Backported to 10.4 docs branch at revision 682853.