aogburn commented on code in PR #882:
URL: https://github.com/apache/tomcat/pull/882#discussion_r2282853608


##########
java/org/apache/catalina/session/FileStore.java:
##########
@@ -217,16 +233,21 @@ public Session load(String id) throws 
ClassNotFoundException, IOException {
     @Override
     public void remove(String id) throws IOException {
         File file = file(id);
-        if (file == null) {
+        if (file == null || !file.exists()) {

Review Comment:
   @n828cl If we lock around that file.exists check, then it seems that 
question just changes slightly to "what if the file is created one CPU cycle 
after the lock is released following the file.exists() call?"  I think the 
expectation with the session manager/file store behavior is that there would 
still be a later session expiration check so if the session file now exists and 
is no longer valid at some point, we'll still have the opportunity to remove 
the file on future checks 
[here](https://github.com/apache/tomcat/blob/main/java/org/apache/catalina/session/StoreBase.java#L169)
 when its expiration has passed.
   
   And in actual practice, I think Tomcat itself should only remove the session 
file through `FileStore.remove`, which could be just from that expiration check 
above or from when an invalid session state is seen in the 
[PersistentManagerBase](https://github.com/apache/tomcat/blob/main/java/org/apache/catalina/session/PersistentManagerBase.java#L579)
 or the 
[PersistentValve](https://github.com/apache/tomcat/blob/main/java/org/apache/catalina/valves/PersistentValve.java#L175).
  If the session is determined invalid and so being removed, I'm not sure how 
there could be a potential state like this would suggest where the session file 
doesn't exist to be removed but then is created for the session after its 
expiration/invalidation.  But nonetheless, any present or future created 
session file should still always have some future expiration check prompting 
for its removal (unless given an indefinite session timeout), correct?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to