Re: Unregistering the driver from DriverManager

Hi,

I've cloned your pull request locally (haven't forked it, even although
I've got a github account and could do so...).

I don't (yet) have time to try it out fully (won't be able to do so until
Tuesday at the earliest), but here's some initial observations.

1/ You might want to use a more recent version of "bnd" (you're using 1.5,
the current is 2.4)

2/ You refer to Bundle-Activator: org.postgresql.osgi.PGBundleActivator in
the manifest, and the source code is there, but it's not included in the
resulting OSGi ".jar" (generated in "/jars").

3/ The changes to the Driver class (register(), unregister(), and
isRegistered()) look good in source code.

4/ In PGBundleActivator::start, you should perhaps use:

Dictionary<String,Object> properties = new Hashtable<>(4);
// instead of "Properties", to avoid implying that values are strings (this
isn't the case here)

5/ In PGBundleActivator::start, shouldn't "Postgresql" be written
"PostgreSQL" in the OSGI_JDBC_DRIVER_NAME property?

6/ In PGBundleActivator::stop, you should set the _registration instance to
null after unregistering because it's possible to restart a stopped bundle.

7/ As you're using Bundle-ManifestVersion: 2, you might also want to
add Require-Capability: osgi.ee;filter:="(&(osgi.ee=JavaSE)(version=1.7))"
(where 1.7 is derived from "ant.java.version" during build), as this
enforces the JDBC 4.1 "level" indirectly by requiring the appropriate
JavaSE version).

The choice of the DataSourceFactory API seems like a good idea to me, as
opposed to just registering the "Driver" interface directly.

Is the 9.4 branch stable (as in safe to use as-is, even if not at "feature
freeze" yet)?

--
Christopher

On 2 January 2015 at 17:08, Alexis Meneses <alexis(dot)meneses(at)gmail(dot)com> wrote:

> Christopher,
>
> (1) Would you mind testing if PR #241
> <https://github.com/pgjdbc/pgjdbc/pull/241> on Github fits your needs and
> works in the OSGi environment you're using?
> Note that it don't check presence of BundleContext via Class.forName()
> because this class could be erroneously present in the classpath even
> without an OSGi Framework being started.
>
> (2) Driver.deregister() method (named after DriverManager method) has been
> exposed so as to be used in other contexts like Java EE.
>
> Alexis
>
>
> 2014-12-29 14:17 GMT+01:00 Christopher BROWN <brown(at)reflexe(dot)fr>:
>
>> There are two main situations where it would be useful to automatically
>> unregister the driver:
>>
>> 1) OSGi - and the suggestion of using BundleActivator.stop() would be a
>> good fit here (as long as care is taken to ensure resolution=optional for
>> other dependencies)
>>
>> 2) Java EE web applications, using a ServletContextListener, perhaps
>> using annotations as described in
>> https://blogs.oracle.com/swchan/entry/servlet_3_0_annotations (but this
>> would exclude older application servers)
>>
>> With regards to (2), I generally place JDBC drivers in the main
>> classloader of the application server, as opposed to embedding in
>> WEB-INF/lib when working with webapps. Also, not all of the webapps I have
>> to deal with (from time to time, it's not my main focus) are up to Servlet
>> 3.0, many as still stuck on 2.5. And in any case, embedding JDBC drivers
>> in webapps (without matching versions) then accessing them via
>> DriverManager is may cause class lookup problems.
>>
>> A good solution to (1) above to me would be like this then (building on
>> the suggestion of Alexis):
>>
>> - keep the static block in driver
>> - check -- via Class.forName("org.osgi.framework.BundleContext") -- if
>> OSGi classes are visible, implying that the driver has been loaded as a
>> bundle, and skip the DriverManager registration in the static block (unless
>> forced via a system property, just in case something breaks for someone
>> relying on current behavior)
>> - register the driver in DriverManager in the BundleActivator.start()
>> method
>> - unregister it (same instance, kept as a reference) in the
>> BundleActivator.stop() method
>>
>> The only reason I mention (2) is because it might be useful to share some
>> common code. Or not. In any case, (1) is the only requirement at this
>> time and (2) isn't as much of a problem.
>>
>> --
>> Christopher
>>
>>
>> On 29 December 2014 at 13:45, Alexis Meneses <alexis(dot)meneses(at)gmail(dot)com>
>> wrote:
>>
>>> If the only concern is OSGi environments, I think that unregistering
>>> could be handled in a BundleActivator
>>> <http://www.osgi.org/javadoc/r4v43/core/org/osgi/framework/BundleActivator.html>.stop()
>>> implementation bundled with the driver.
>>>
>>> See pending issue #71 <https://github.com/pgjdbc/pgjdbc/issues/71> on
>>> Github.
>>>
>>>
>>> 2014-12-29 12:48 GMT+01:00 Dave Cramer <pg(at)fastcrypt(dot)com>:
>>>
>>>> I have no objection to an unregister static method being added. It's
>>>> not in the API so it would not effect anything really
>>>>
>>>> Dave Cramer
>>>>
>>>> dave.cramer(at)credativ(dot)ca
>>>> http://www.credativ.ca
>>>>
>>>> On 29 December 2014 at 04:53, Christopher BROWN <brown(at)reflexe(dot)fr>
>>>> wrote:
>>>>
>>>>> Hello,
>>>>>
>>>>> I'm starting to integrate the Postgresql JDBC driver into an OSGi
>>>>> environment, as an OSGi bundle. I'm evaluating the different ways to avoid
>>>>> a classloader leak with DriverManager when hot-swapping the driver bundle
>>>>> without restarting the host application, and am seeking suggestions on best
>>>>> practice regarding the Postgresql JDBC driver.
>>>>>
>>>>> Another bundle (which I provide, it's not third-party) will directly
>>>>> depend upon it (loading classes directly, namely org.postgresql.Driver);
>>>>> when the Postgresql JDBC driver classes are loaded, the other bundle will
>>>>> create a DataSource using a JDBC connection pool, and register the
>>>>> DataSource as an OSGi service. Normally, that's all that will happen
>>>>> during the application lifecycle, but in principle, it's possible for the
>>>>> administrator to want to replace say the 9.3 driver with the 9.4 driver by
>>>>> removing the 9.3 ".jar" at runtime, and replacing it with the 9.4 ".jar",
>>>>> all at runtime; when the first ".jar" is deleted, the dependent bundle is
>>>>> knocked offline, unregistering the DataSource automatically, and notifying
>>>>> all clients; when the second is installed, the application is once again
>>>>> fully-functional (and all this normally occurs within a few hundred
>>>>> milliseconds).
>>>>>
>>>>> Looking at the source code, I can see that the org.postgresql.Driver
>>>>> class registers itself in a "static" block with DriverManager (which is the
>>>>> correct behavior regarding the JDBC spec). However, short of a brute-force
>>>>> loop -- like this one: http://stackoverflow.com/a/5315467 (enhanced
>>>>> to check the class name of each driver, to avoid clobbering unrelated
>>>>> driver registrations) -- is there any other approach possible or that could
>>>>> be added, say a NonRegisteringDriver (superclass of Driver, with all logic
>>>>> except for the static initializer) or an "unregister()" static method, or a
>>>>> field containing the registered Driver instance?
>>>>>
>>>>> Thanks,
>>>>> Christopher
>>>>>
>>>>>
>>>>
>>>
>>
>