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; - } }
