Re: Unregistering the driver from DriverManager

Thanks for your feedback. All your observations make sense to me and I
updated the PR branch accordingly.

Alexis

2015-01-03 15:30 GMT+01:00 Christopher BROWN <brown(at)reflexe(dot)fr>:

> 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
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>