On 8/16/06, Garrett Rooney <[EMAIL PROTECTED]> wrote:
On 8/15/06, William A. Rowe, Jr. <[EMAIL PROTECTED]> wrote:
> Branko Čibej wrote:
> > Joe Orton wrote:
> >> I'm very much unconvinced. The app can serialize access to the
> >> APR-global pool, if it wants to - likewise it could create a special
> >> "DSO-holding-pool" from the global pool directly after calling
> >> apr_initialize() to avoid the whole issue.
> >>
> >
> > You're still thinking in terms of "the app". Subversion is not an
> > application. Yes, we could add an svn_initialize function that should be
> > called by the app that uses SVN libraries right after apr_initialize.
> > But our API compatibility guarantees (just like APR's) mean we can't
> > force the use of that function until svn-2.0. Also, there's a large code
> > base using SVN libs that have to work with newer releases.
> >
> > Granted, we made a mistake in our API. That doesn't mean there's no
> > usability problem in APR.
>
> I grok where you are getting at (remember that a program may load an apr
> based module without even being an apr application Joe! Worse, it may
> load two such modules).
>
> Branko - would you like to add a simple apr/test/ app that illustrates
> the issue for your skeptics? Then it will be simpler to fix the API if
> necessary or provide the appropriate workarounds, and document that through
> the test example.
I'll throw something together that demonstrates the problem.
Here's a patch to mod_test.c and testdso.c that illustrates the
problem. Note that this is running inside the test framework, so it's
not identical to the case we're talking about, but you should be able
to get the idea from what's there. If we move forward on this I'll
move the code out into a separate program so we can really simulate
the case we're talking about.
-garrett
Index: test/mod_test.c
===================================================================
--- test/mod_test.c (revision 432033)
+++ test/mod_test.c (working copy)
@@ -30,3 +30,13 @@
for (i = 0;i < reps; i++);
return i;
}
+
+static apr_status_t callback(void *data)
+{
+ /* Nothing here... */
+}
+
+void set_callback(apr_pool_t *pool)
+{
+ apr_pool_cleanup_register(pool, NULL, callback, apr_pool_cleanup_null);
+}
Index: test/testdso.c
===================================================================
--- test/testdso.c (revision 432033)
+++ test/testdso.c (working copy)
@@ -219,6 +219,47 @@
ABTS_INT_EQUAL(tc, 1, APR_STATUS_IS_ESYMNOTFOUND(status));
}
+typedef void (*set_callback_t)(apr_pool_t *);
+
+static void test_global_problem(abts_case *tc, void *data)
+{
+ apr_pool_t *global_pool, *dso_pool, *app_pool;
+ apr_dso_handle_sym_t func = NULL;
+ set_callback_t set_callback;
+ apr_dso_handle_t *h = NULL;
+ apr_status_t status;
+ char errstr[256];
+
+ /* This simulates the APR "global pool" */
+ apr_pool_create(&global_pool, NULL);
+
+ /* Then assume that the user creates his own application pool. */
+ apr_pool_create(&app_pool, global_pool);
+
+ /* Then you call into some function that tries to load a DSO into its
+ * own "global" pool, which is created after the app's global pool. */
+ apr_pool_create(&dso_pool, global_pool);
+
+ /* Load into the dso pool. */
+ status = apr_dso_load(&h, libname, dso_pool);
+ ABTS_ASSERT(tc, apr_dso_error(h, errstr, 256), APR_SUCCESS == status);
+ ABTS_PTR_NOTNULL(tc, h);
+
+ status = apr_dso_sym(&func, h, "set_callback");
+ ABTS_ASSERT(tc, apr_dso_error(h, errstr, 256), APR_SUCCESS == status);
+
+ set_callback = (set_callback_t) func;
+
+ /* Now call a function you got from the DSO, passing in the application
+ * pool. The function registers a cleanup callback on it which is defined
+ * in the DSO. */
+ set_callback(app_pool);
+
+ /* Now we simulate shutting down the application by destroying the global
+ * pool. The pools will be destroyed in an order that results in calling
+ * the cleanup after its DSO has been unloaded. */
+ apr_pool_destroy(global_pool);
+}
#endif /* def(LIB_NAME) */
static void test_load_notthere(abts_case *tc, void *data)
@@ -253,6 +294,8 @@
abts_run_test(suite, test_dso_sym_library, NULL);
abts_run_test(suite, test_dso_sym_return_value_library, NULL);
abts_run_test(suite, test_unload_library, NULL);
+
+ abts_run_test(suite, test_global_problem, NULL);
#endif
abts_run_test(suite, test_load_notthere, NULL);
