0003446: Need post-send shutdown functions

(0003969)
koreth
04-28-09 22:54

Working implementation attached. It is probably kind of lame -- it breaks encapsulation a bit, but I didn't immediately see a cleaner way to do it.

(0004011)
koreth
05-13-09 16:21

My patch is not quite right; need to move the cleanup handlers below the post-send handlers. Patch to the patched code base (this is just a simple cut-and-paste of the code in question):

--- a/modules/quercus/src/com/caucho/quercus/env/Env.java
+++ b/modules/quercus/src/com/caucho/quercus/env/Env.java
@@ -6354,23 +6354,6 @@ public class Env {
     } catch (Throwable e) {
       log.log(Level.FINE, e.toString(), e);
     }
-
- if (_cleanupList != null) {
- ArrayList<EnvCleanup> cleanupList
- = new ArrayList<EnvCleanup>(_cleanupList);
-
- // cleanup is in reverse order of creation
- for (int i = cleanupList.size() - 1; i >= 0; i--) {
- EnvCleanup envCleanup = cleanupList.get(i);
- try {
- if (envCleanup != null)
- envCleanup.cleanup();
- }
- catch (Throwable e) {
- log.log(Level.FINER, e.toString(), e);
- }
- }
- }
   }

   /**
@@ -6400,6 +6383,22 @@ public class Env {
       }
     }

+ if (_cleanupList != null) {
+ ArrayList<EnvCleanup> cleanupList
+ = new ArrayList<EnvCleanup>(_cleanupList);
+
+ // cleanup is in reverse order of creation
+ for (int i = cleanupList.size() - 1; i >= 0; i--) {
+ EnvCleanup envCleanup = cleanupList.get(i);
+ try {
+ if (envCleanup != null)
+ envCleanup.cleanup();
+ }
+ catch (Throwable e) {
+ log.log(Level.FINER, e.toString(), e);
+ }
+ }
+ }

     _threadEnv.set(_oldThreadEnv);

(0004012)
nam
05-13-09 18:05

It's probably not a good idea for the servlet to close the connection to the client (i.e. services and frameworks like Comet might depend on the connection being open).

An alternative solution is to put asynchronous processing at the end of a request onto a BAM queue. For more info on BAM, see http://blog.caucho.com/?p=163 [^]

(0004013)
koreth
05-13-09 19:48

Can the BAM approach maintain the execution context of the script? The idea with these postsend handlers is that they are just like shutdown handlers: they have full access to the request data, global variables, constant/function definitions, etc. My understanding of the BAM setup is that you'd have to essentially dispatch a completely new request, which would somewhat defeat the purpose of the feature.

In any case, I think one could reasonably enough lay down the requirement that persistent connections and postsend handlers aren't allowed to mix. It doesn't make any sense to mix them semantically anyway since the whole point is to do stuff after the client connection is released (thus allowing the client to open subsequent connections to load other resources on the page while the server continues to do cleanup work), and if the client connection isn't going to close regardless, you may as well just do the work in a shutdown handler since the user-visible effect will be identical. It would probably make sense to detect the combination of the two and spit out an error.

Issue History
Date Modified	Username	Field	Change
04-15-09 12:59	koreth	New Issue
04-28-09 22:53	koreth	File Added: patch.txt
04-28-09 22:54	koreth	Note Added: 0003969
05-13-09 16:21	koreth	Note Added: 0004011
05-13-09 18:05	nam	Note Added: 0004012
05-13-09 19:48	koreth	Note Added: 0004013

Notes
(0003969) koreth 04-28-09 22:54	Working implementation attached. It is probably kind of lame -- it breaks encapsulation a bit, but I didn't immediately see a cleaner way to do it.

(0004011) koreth 05-13-09 16:21	My patch is not quite right; need to move the cleanup handlers below the post-send handlers. Patch to the patched code base (this is just a simple cut-and-paste of the code in question): --- a/modules/quercus/src/com/caucho/quercus/env/Env.java +++ b/modules/quercus/src/com/caucho/quercus/env/Env.java @@ -6354,23 +6354,6 @@ public class Env { } catch (Throwable e) { log.log(Level.FINE, e.toString(), e); } - - if (_cleanupList != null) { - ArrayList<EnvCleanup> cleanupList - = new ArrayList<EnvCleanup>(_cleanupList); - - // cleanup is in reverse order of creation - for (int i = cleanupList.size() - 1; i >= 0; i--) { - EnvCleanup envCleanup = cleanupList.get(i); - try { - if (envCleanup != null) - envCleanup.cleanup(); - } - catch (Throwable e) { - log.log(Level.FINER, e.toString(), e); - } - } - } } /** @@ -6400,6 +6383,22 @@ public class Env { } } + if (_cleanupList != null) { + ArrayList<EnvCleanup> cleanupList + = new ArrayList<EnvCleanup>(_cleanupList); + + // cleanup is in reverse order of creation + for (int i = cleanupList.size() - 1; i >= 0; i--) { + EnvCleanup envCleanup = cleanupList.get(i); + try { + if (envCleanup != null) + envCleanup.cleanup(); + } + catch (Throwable e) { + log.log(Level.FINER, e.toString(), e); + } + } + } _threadEnv.set(_oldThreadEnv);

(0004012) nam 05-13-09 18:05	It's probably not a good idea for the servlet to close the connection to the client (i.e. services and frameworks like Comet might depend on the connection being open). An alternative solution is to put asynchronous processing at the end of a request onto a BAM queue. For more info on BAM, see http://blog.caucho.com/?p=163 [^]

(0004013) koreth 05-13-09 19:48	Can the BAM approach maintain the execution context of the script? The idea with these postsend handlers is that they are just like shutdown handlers: they have full access to the request data, global variables, constant/function definitions, etc. My understanding of the BAM setup is that you'd have to essentially dispatch a completely new request, which would somewhat defeat the purpose of the feature. In any case, I think one could reasonably enough lay down the requirement that persistent connections and postsend handlers aren't allowed to mix. It doesn't make any sense to mix them semantically anyway since the whole point is to do stuff after the client connection is released (thus allowing the client to open subsequent connections to load other resources on the page while the server continues to do cleanup work), and if the client connection isn't going to close regardless, you may as well just do the work in a shutdown handler since the user-visible effect will be identical. It would probably make sense to detect the combination of the two and spit out an error.

Relationships