ws-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From veit...@apache.org
Subject svn commit: r1310029 - /webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/attachments/PartContentFactory.java
Date Thu, 05 Apr 2012 19:33:21 GMT
Author: veithen
Date: Thu Apr  5 19:33:21 2012
New Revision: 1310029

URL: http://svn.apache.org/viewvc?rev=1310029&view=rev
Log:
Removed the code that attempts to dynamically adjusts the file threshold (depending on available
memory) and that limits the concurrency in the createPartContent method.

Reasons:

1. That code seems to be responsible for sporadically occurring build failures, as reported
in the following two posts on the mailing list:

http://markmail.org/thread/eclcydt6kq3ygcot
http://markmail.org/message/ikr6pxvblz6vn4qc

2. The code limits the concurrency (to a maximum of 4 parallel executions) of a piece of code
that contains blocking calls that may need to wait for data from a remote client. Depending
on how Axiom is used, this may be exploited by a malicious client to cause a denial of service.

3. The "feature" is completely undocumented and nothing in the API gives a hint about its
existence. Only people reading the more obscure parts of the Axiom code would be aware of
it.

4. The algorithm that calculates the dynamic threshold uses Runtime#freeMemory() as variable.
Because of the garbage collector, that variable is highly volatile and the feature doesn't
work in a deterministic way. E.g. if the code is executed shortly before a garbage collection
occurs, freeMemory will be small which may lead to unexpected results.

5. The code was contributed by IBM and was likely optimized for usage in a specific product.
The feature uses several fixed parameters and it is unlikely that this set of parameters are
appropriate in other scenarios.

6. From a design perspective, Axiom is not the right place for this kind of code, because
Axiom may be used in a variety of use cases, but the code is tailored for a specific scenario.
Axiom may define interfaces that allow to plug in this kind of features, but the implementation
should be provided by the SOAP stack or the application server, not hardwired into Axiom.

Modified:
    webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/attachments/PartContentFactory.java

Modified: webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/attachments/PartContentFactory.java
URL: http://svn.apache.org/viewvc/webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/attachments/PartContentFactory.java?rev=1310029&r1=1310028&r2=1310029&view=diff
==============================================================================
--- webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/attachments/PartContentFactory.java
(original)
+++ webservices/commons/trunk/modules/axiom/modules/axiom-api/src/main/java/org/apache/axiom/attachments/PartContentFactory.java
Thu Apr  5 19:33:21 2012
@@ -35,19 +35,8 @@ import java.io.InputStream;
  * makes it easier to add new implementations.
  */
 class PartContentFactory {
-    
-    private static int inflight = 0;  // How many attachments are currently being built.
-    private static final String semaphore = "PartContentFactory.semaphore";
-    
     private static final Log log = LogFactory.getLog(PartContentFactory.class);
     
-    // Maximum number of threads allowed through createPart
-    private static final int INFLIGHT_MAX = 4;
-    
-    // Constants for dynamic threshold 
-    // Dynamic Threshold = availMemory / THRESHOLD_FACTOR
-    private static final int THRESHOLD_FACTOR = 5;
-    
     /**
      * Creates a {@link PartContent} object from a given input stream. The remaining parameters
are
      * used to determine if the content should be stored in memory (byte buffers) or backed
by a
@@ -78,136 +67,41 @@ class PartContentFactory {
         }
         
         try {
-            PartContent partContent;
-            try {
-                
-                // Message throughput is increased if the number of threads in this
-                // section is limited to INFLIGHT_MAX.  Allowing more threads tends to cause
-                // thrashing while reading from the HTTP InputStream.  
-                // Allowing fewer threads reduces the thrashing.  And when the remaining
threads
-                // are notified their input (chunked) data is available.
-                // 
-                // Note: the root part is at the beginning of the message and much smaller
than attachments,
-                // so don't wait on root parts.
-                if (!isRootPart) {
-                    synchronized(semaphore) {
-                        if (inflight >= INFLIGHT_MAX) {
-                            semaphore.wait();
-                        }
-                        inflight++;
-                    }
-                }
-                // Get new threshold based on the current available memory in the runtime.
-                // We only use the thresholds for non-root parts.
-                if (!isRootPart && thresholdSize > 0) {     
-                    thresholdSize = getRuntimeThreshold(thresholdSize, inflight);
-                }
+            if (isRootPart ||
+                    thresholdSize <= 0 ||  
+                    (messageContentLength > 0 && 
+                            messageContentLength < thresholdSize)) {
+                // If the entire message is less than the threshold size, 
+                // keep it in memory.
+                // If this is the root part, keep it in memory.
+
+                // Get the bytes of the data without a lot 
+                // of resizing and GC.  The BAAOutputStream 
+                // keeps the data in non-contiguous byte buffers.
+                BAAOutputStream baaos = new BAAOutputStream();
+                BufferUtils.inputStream2OutputStream(in, baaos);
+                return new PartContentOnMemory(baaos.buffers(), baaos.length());
+            } else {
+                // We need to read the input stream to determine whether
+                // the size is bigger or smaller than the threshold.
+                BAAOutputStream baaos = new BAAOutputStream();
+                int count = BufferUtils.inputStream2OutputStream(in, baaos, thresholdSize);
 
-                
-                if (isRootPart ||
-                        thresholdSize <= 0 ||  
-                        (messageContentLength > 0 && 
-                                messageContentLength < thresholdSize)) {
-                    // If the entire message is less than the threshold size, 
-                    // keep it in memory.
-                    // If this is the root part, keep it in memory.
-
-                    // Get the bytes of the data without a lot 
-                    // of resizing and GC.  The BAAOutputStream 
-                    // keeps the data in non-contiguous byte buffers.
-                    BAAOutputStream baaos = new BAAOutputStream();
-                    BufferUtils.inputStream2OutputStream(in, baaos);
-                    partContent = new PartContentOnMemory(baaos.buffers(), baaos.length());
+                if (count < thresholdSize) {
+                    return new PartContentOnMemory(baaos.buffers(), baaos.length());
                 } else {
-                    // We need to read the input stream to determine whether
-                    // the size is bigger or smaller than the threshold.
-                    BAAOutputStream baaos = new BAAOutputStream();
-                    int count = BufferUtils.inputStream2OutputStream(in, baaos, thresholdSize);
-
-                    if (count < thresholdSize) {
-                        partContent = new PartContentOnMemory(baaos.buffers(), baaos.length());
-                    } else {
-                        // A BAAInputStream is an input stream over a list of non-contiguous
4K buffers.
-                        BAAInputStream baais = 
-                            new BAAInputStream(baaos.buffers(), baaos.length());
-
-                        partContent = new PartContentOnFile(manager, 
-                                              baais,
-                                              in, 
-                                              attachmentDir);
-                    }
-
-                } 
-            } finally {
-                if (!isRootPart) {
-                    synchronized(semaphore) {
-                        semaphore.notify();
-                        inflight--;
-                    }
+                    // A BAAInputStream is an input stream over a list of non-contiguous
4K buffers.
+                    BAAInputStream baais = 
+                        new BAAInputStream(baaos.buffers(), baaos.length());
+
+                    return new PartContentOnFile(manager, 
+                                          baais,
+                                          in, 
+                                          attachmentDir);
                 }
-            }
-
-            return partContent;
-            
+            } 
         } catch (Exception e) {
             throw new OMException(e);
         } 
     }
-    
-    /**
-     * This method checks the configured threshold and
-     * the current runtime information.  If it appears that we could
-     * run out of memory, the threshold is reduced.
-     * 
-     * This method allows the user to request a much larger threshold without 
-     * fear of running out of memory.  Using a larger in memory threshold generally 
-     * results in better throughput.
-     * 
-     * @param configThreshold
-     * @param inflight
-     * @return threshold
-     */
-    private static int getRuntimeThreshold(int configThreshold, int inflight) {
-        
-        // Determine how much free memory is available
-        Runtime r = Runtime.getRuntime();
-        long totalmem = r.totalMemory();
-        long maxmem = r.maxMemory();
-        long freemem = r.freeMemory();
-        
-        // @REVIEW
-        // If maximum is not defined...limit to 1G
-        if (maxmem == java.lang.Long.MAX_VALUE) {
-            maxmem = 1024*1024*1024; 
-        }
-        
-        long availmem = maxmem - (totalmem - freemem);
-        
-       
-        // Now determine the dynamic threshold
-        int dynamicThreshold = (int) availmem / (THRESHOLD_FACTOR * inflight);
-        
-        // If it appears that we might run out of memory with this
-        // threshold, reduce the threshold size.
-        if (dynamicThreshold < configThreshold) {
-            if (log.isDebugEnabled()) {
-                log.debug("Using Runtime Attachment File Threshold " + dynamicThreshold);
-                log.debug("maxmem   = " + maxmem);
-                log.debug("totalmem = " + totalmem);
-                log.debug("freemem  = " + freemem);
-                log.debug("availmem = " + availmem);
-            }
-            
-        } else {
-            dynamicThreshold = configThreshold;
-            if (log.isDebugEnabled()) {
-                log.debug("Using Configured Attachment File Threshold " + configThreshold);
-                log.debug("maxmem   = " + maxmem);
-                log.debug("totalmem = " + totalmem);
-                log.debug("freemem  = " + freemem);
-                log.debug("availmem = " + availmem);
-            }
-        }
-        return dynamicThreshold;
-    }
 }



Mime
View raw message