You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2005/05/11 13:23:26 UTC
cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11 Http11AprProcessor.java
remm 2005/05/11 04:23:26
Modified: util/java/org/apache/tomcat/util/net AprEndpoint.java
http11/src/java/org/apache/coyote/http11
Http11AprProcessor.java
Log:
- Should fix thread safety issue reported by Bill (needs testing).
Revision Changes Path
1.25 +63 -32 jakarta-tomcat-connectors/util/java/org/apache/tomcat/util/net/AprEndpoint.java
Index: AprEndpoint.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-connectors/util/java/org/apache/tomcat/util/net/AprEndpoint.java,v
retrieving revision 1.24
retrieving revision 1.25
diff -u -r1.24 -r1.25
--- AprEndpoint.java 3 May 2005 09:36:58 -0000 1.24
+++ AprEndpoint.java 11 May 2005 11:23:26 -0000 1.25
@@ -17,6 +17,7 @@
package org.apache.tomcat.util.net;
import java.net.InetAddress;
+import java.util.ArrayList;
import java.util.HashMap;
import java.util.Stack;
@@ -715,6 +716,10 @@
protected long pool = 0;
protected long[] desc;
+ protected long[] addS;
+ protected long[] addP;
+ protected int addCount = 0;
+
protected synchronized void init() {
pool = Pool.create(serverSockPool);
try {
@@ -735,6 +740,8 @@
}
desc = new long[pollerSize * 4];
keepAliveCount = 0;
+ addS = new long[pollerSize];
+ addP = new long[pollerSize];
}
protected void destroy() {
@@ -742,23 +749,17 @@
}
public void add(long socket, long pool) {
- synchronized (this) {
- int rv = Poll.add(serverPollset, socket, pool, Poll.APR_POLLIN);
- if (rv == Status.APR_SUCCESS) {
- keepAliveCount++;
- } else {
+ synchronized (addS) {
+ // Add socket to the list. Newly added sockets will wait
+ // at most for pollTime before being polled
+ if (addCount >= addS.length) {
// Can't do anything: close the socket right away
Pool.destroy(pool);
+ return;
}
- }
- }
-
- public void remove(long socket) {
- synchronized (this) {
- int rv = Poll.remove(serverPollset, socket);
- if (rv == Status.APR_SUCCESS) {
- keepAliveCount--;
- }
+ addS[addCount] = socket;
+ addP[addCount] = pool;
+ addCount++;
}
}
@@ -780,7 +781,7 @@
}
}
- while (keepAliveCount < 1) {
+ while (keepAliveCount < 1 && addCount < 1) {
try {
Thread.sleep(10);
} catch (InterruptedException e) {
@@ -789,6 +790,22 @@
}
try {
+ // Add sockets which are waiting to the poller
+ if (addCount > 0) {
+ synchronized (addS) {
+ for (int i = 0; i < addCount; i++) {
+ int rv = Poll.add
+ (serverPollset, addS[i], addP[i], Poll.APR_POLLIN);
+ if (rv == Status.APR_SUCCESS) {
+ keepAliveCount++;
+ } else {
+ // Can't do anything: close the socket right away
+ Pool.destroy(pool);
+ }
+ }
+ addCount = 0;
+ }
+ }
// Pool for the specified interval
int rv = Poll.poll(serverPollset, pollTime, desc, true);
if (rv > 0) {
@@ -971,7 +988,8 @@
// Range information
public long start;
public long end;
- // Socket pool
+ // Socket and socket pool
+ public long socket;
public long pool;
// Position
public long pos;
@@ -990,6 +1008,9 @@
protected long pool = 0;
protected long[] desc;
protected HashMap sendfileData;
+
+ protected ArrayList addS;
+
protected void init() {
pool = Pool.create(serverSockPool);
try {
@@ -1010,6 +1031,7 @@
}
desc = new long[sendfileSize * 4];
sendfileData = new HashMap(sendfileSize);
+ addS = new ArrayList();
}
protected void destroy() {
@@ -1017,7 +1039,7 @@
Pool.destroy(pool);
}
- public boolean add(long socket, SendfileData data) {
+ public boolean add(SendfileData data) {
// Initialize fd from data given
try {
data.fdpool = Pool.create(data.pool);
@@ -1027,9 +1049,9 @@
0, data.fdpool);
data.pos = data.start;
// Set the socket to nonblocking mode
- Socket.optSet(socket, Socket.APR_SO_NONBLOCK, 1);
+ Socket.optSet(data.socket, Socket.APR_SO_NONBLOCK, 1);
while (true) {
- long nw = Socket.sendfile(socket, data.fd, null, null,
+ long nw = Socket.sendfile(data.socket, data.fd, null, null,
data.pos, data.end, 0);
if (nw < 0) {
if (!Status.APR_STATUS_IS_EAGAIN((int) -nw)) {
@@ -1045,7 +1067,7 @@
// Entire file has been send
Poll.destroy(data.fdpool);
// Set back socket to blocking mode
- Socket.optSet(socket, Socket.APR_SO_NONBLOCK, 0);
+ Socket.optSet(data.socket, Socket.APR_SO_NONBLOCK, 0);
return true;
}
}
@@ -1054,17 +1076,10 @@
log.error(sm.getString("endpoint.sendfile.error"), e);
return false;
}
- synchronized (this) {
- // Add socket to the poller
- sendfileData.put(new Long(socket), data);
- int rv = Poll.add(sendfilePollset, socket, 0, Poll.APR_POLLOUT);
- if (rv == Status.APR_SUCCESS) {
- sendfileCount++;
- } else {
- log.warn(sm.getString("endpoint.sendfile.addfail", "" + rv));
- // Can't do anything: close the socket right away
- Pool.destroy(data.pool);
- }
+ // Add socket to the list. Newly added sockets will wait
+ // at most for pollTime before being polled
+ synchronized (addS) {
+ addS.add(sendfileData);
}
return false;
}
@@ -1099,7 +1114,7 @@
}
}
- while (sendfileCount < 1) {
+ while (sendfileCount < 1 && addS.size() < 1) {
try {
Thread.sleep(10);
} catch (InterruptedException e) {
@@ -1108,6 +1123,22 @@
}
try {
+ // Add socket to the poller
+ if (addS.size() > 0) {
+ for (int i = 0; i < addS.size(); i++) {
+ SendfileData data = (SendfileData) addS.get(i);
+ int rv = Poll.add(sendfilePollset, data.socket, 0, Poll.APR_POLLOUT);
+ if (rv == Status.APR_SUCCESS) {
+ sendfileData.put(new Long(data.socket), data);
+ sendfileCount++;
+ } else {
+ log.warn(sm.getString("endpoint.sendfile.addfail", "" + rv));
+ // Can't do anything: close the socket right away
+ Pool.destroy(data.pool);
+ }
+ }
+ addS.clear();
+ }
// Pool for the specified interval
int rv = Poll.poll(sendfilePollset, pollTime, desc, false);
if (rv > 0) {
1.11 +2 -1 jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11/Http11AprProcessor.java
Index: Http11AprProcessor.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11/Http11AprProcessor.java,v
retrieving revision 1.10
retrieving revision 1.11
diff -u -r1.10 -r1.11
--- Http11AprProcessor.java 28 Apr 2005 12:30:38 -0000 1.10
+++ Http11AprProcessor.java 11 May 2005 11:23:26 -0000 1.11
@@ -872,8 +872,9 @@
// Do sendfile as needed: add socket to sendfile and end
if (sendfileData != null) {
+ sendfileData.socket = socket;
sendfileData.pool = pool;
- if (!endpoint.getSendfile().add(socket, sendfileData)) {
+ if (!endpoint.getSendfile().add(sendfileData)) {
keepAlive = false;
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11AprProcessor.java
Posted by Mladen Turk <mt...@apache.org>.
Remy Maucherat wrote:
> Bill Barker wrote:
>> if(useSendfile && !Apr.hasFeature(Apr.SENDFILE)) {
>> log.warn("disabling sendfile, since your apr version doesn't
>> support it");
>> useSendfile = false;
>> }
>
> I didn't know that. Yes, the check is needed then.
>
Yes seems it's needed.
You already have the :
Library.APR_HAS_SENDFILE boolean, that is meant to be used
for that (checking if the OS supports sendfile).
Regards,
Mladen.
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11AprProcessor.java
Posted by Remy Maucherat <re...@apache.org>.
Bill Barker wrote:
> Peter's test was stressing my poor little box a lot (which it really
> shouldn't, since it is a high-concurrency/low-traffic test). It was one
> of the things I wanted to look into when I had time to run more tests.
>
> Also, I've been running the test with sendfile disabled. It seems that
> Solaris7 doesn't have this particular feature :(It was added to
> Solaris8). With sendfile enabled, I get a nice core-dump (since there is
> no apr_socket_sendfile in my libapr.so :). I'm guessing that a for
> production release we need something like this in AprEndpoint.init:
> if(useSendfile && !Apr.hasFeature(Apr.SENDFILE)) {
> log.warn("disabling sendfile, since your apr version doesn't
> support it");
> useSendfile = false;
> }
I didn't know that. Yes, the check is needed then.
>> He still needs to remove some syncs in the native code, apparently (=
>> the hacks he added on top of APR, but which didn't quite work).
>
> It seems I need to look for a few more patches before I can usefully test.
Which changes do you suggest besides fixing sendfile ?
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11 Http11AprProcessor.java
Posted by Bill Barker <wb...@wilshire.com>.
----- Original Message -----
From: "Remy Maucherat" <re...@apache.org>
To: "Tomcat Developers List" <to...@jakarta.apache.org>
Sent: Friday, May 13, 2005 9:29 PM
Subject: Re: cvs commit:
jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11AprProcessor.java
>Bill Barker wrote:
>>>remm 2005/05/11 04:23:26
>>>
>>> Modified: util/java/org/apache/tomcat/util/net AprEndpoint.java
>>> http11/src/java/org/apache/coyote/http11
>>> Http11AprProcessor.java
>>> Log:
>>> - Should fix thread safety issue reported by Bill (needs testing).
>>
>> Peter's test case does finish with this on my Solaris box. I'll see if I
>> can do more testing next week.
>
>Mladen told me that the sync was badly done, and that I should be using a
>queue instead (too bad, it took one week).
Peter's test was stressing my poor little box a lot (which it really
shouldn't, since it is a high-concurrency/low-traffic test). It was one of
the things I wanted to look into when I had time to run more tests.
Also, I've been running the test with sendfile disabled. It seems that
Solaris7 doesn't have this particular feature :(It was added to Solaris8).
With sendfile enabled, I get a nice core-dump (since there is no
apr_socket_sendfile in my libapr.so :). I'm guessing that a for production
release we need something like this in AprEndpoint.init:
if(useSendfile && !Apr.hasFeature(Apr.SENDFILE)) {
log.warn("disabling sendfile, since your apr version doesn't
support it");
useSendfile = false;
}
>
>He still needs to remove some syncs in the native code, apparently (= the
>hacks he added on top of APR, but which didn't quite work).
>
It seems I need to look for a few more patches before I can usefully test.
>Rémy
This message is intended only for the use of the person(s) listed above as the intended recipient(s), and may contain information that is PRIVILEGED and CONFIDENTIAL. If you are not an intended recipient, you may not read, copy, or distribute this message or any attachment. If you received this communication in error, please notify us immediately by e-mail and then delete all copies of this message and any attachments.
In addition you should be aware that ordinary (unencrypted) e-mail sent through the Internet is not secure. Do not send confidential or sensitive information, such as social security numbers, account numbers, personal identification numbers and passwords, to us via ordinary (unencrypted) e-mail.
Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11AprProcessor.java
Posted by Remy Maucherat <re...@apache.org>.
Bill Barker wrote:
>>remm 2005/05/11 04:23:26
>>
>> Modified: util/java/org/apache/tomcat/util/net AprEndpoint.java
>> http11/src/java/org/apache/coyote/http11
>> Http11AprProcessor.java
>> Log:
>> - Should fix thread safety issue reported by Bill (needs testing).
>
> Peter's test case does finish with this on my Solaris box. I'll see if I
> can do more testing next week.
Mladen told me that the sync was badly done, and that I should be using
a queue instead (too bad, it took one week).
He still needs to remove some syncs in the native code, apparently (=
the hacks he added on top of APR, but which didn't quite work).
Rémy
---------------------------------------------------------------------
To unsubscribe, e-mail: tomcat-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: tomcat-dev-help@jakarta.apache.org
Re: cvs commit: jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11 Http11AprProcessor.java
Posted by Bill Barker <wb...@wilshire.com>.
----- Original Message -----
From: <re...@apache.org>
To: <ja...@apache.org>
Sent: Wednesday, May 11, 2005 4:23 AM
Subject: cvs commit:
jakarta-tomcat-connectors/http11/src/java/org/apache/coyote/http11
Http11AprProcessor.java
> remm 2005/05/11 04:23:26
>
> Modified: util/java/org/apache/tomcat/util/net AprEndpoint.java
> http11/src/java/org/apache/coyote/http11
> Http11AprProcessor.java
> Log:
> - Should fix thread safety issue reported by Bill (needs testing).
Peter's test case does finish with this on my Solaris box. I'll see if I
can do more testing next week.
This message is intended only for the use of the person(s) listed above as the intended recipient(s), and may contain information that is PRIVILEGED and CONFIDENTIAL. If you are not an intended recipient, you may not read, copy, or distribute this message or any attachment. If you received this communication in error, please notify us immediately by e-mail and then delete all copies of this message and any attachments.
In addition you should be aware that ordinary (unencrypted) e-mail sent through the Internet is not secure. Do not send confidential or sensitive information, such as social security numbers, account numbers, personal identification numbers and passwords, to us via ordinary (unencrypted) e-mail.