Re: Unregistering the driver from DriverManager

Christopher,

I have already pushed this PR into the main driver

Dave Cramer

dave.cramer(at)credativ(dot)ca
http://www.credativ.ca

On 6 January 2015 at 05:33, Christopher BROWN <brown(at)reflexe(dot)fr> wrote:

> Hello,
>
> I'm trying out the PR branch just now. Seems fine (starts as a bundle,
> except for remark below, connection available, can use new features such as
> connection.getSchema() => "$user") ; a few small remaining observations:
>
> - The dependency on org.osgi.service.jdbc has "resolution:=optional"
> in the MANIFEST.MF file, but it should almost certainly *not* be optional
> (as this causes java.lang.NoClassDefFoundError:
> org/osgi/service/jdbc/DataSourceFactory).
>
> - In the PGBundleActivator class, for stop-restart cases, the "start"
> method should almost certainly have a small conditional block, such as:
>
> if (!Driver.isRegistered()) { Driver.register(); }
>
> => this would mirror the deregistration in the "stop" method, and
> avoid breaking code that might rely on the current static initialization
> behavior.
>
> I'm still building from the forked PR repository ; from your other
> messages, I assume this will soon be merged into the main "pgjdbc"
> repository.
>
> --
> Christopher
>
>
> On 4 January 2015 at 17:37, Alexis Meneses <alexis(dot)meneses(at)gmail(dot)com>
> wrote:
>
>> 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
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>