Permalink
Browse files

Fix for JRUBY-4764: Leaking fileno to ChannelDescriptor mappings in R…

…uby.retainedDescriptors.

This change remains on master, but I'm reverting it on 1.5 because the old weak map is a known quantity that worked for years. We'll continue to explore a hard map + finalizers on master for 1.6, and I'll file a bug for that work.

Revert "No need to use a weak map for fileno to ChannelDescriptor, since in C land unclosed fd's live forever too. Eliminates special need for a hard map for sysopen and weak reference management for all other IOs."

This reverts commit 0c012c7.

Conflicts:

	src/org/jruby/Ruby.java
  • Loading branch information...
1 parent f3e08e0 commit 4760d0acec1a14cbe0e3fab34d488e195faf1b98 @headius headius committed Apr 29, 2010
Showing with 33 additions and 3 deletions.
  1. +33 −3 src/org/jruby/Ruby.java
View
@@ -128,6 +128,7 @@
import com.kenai.constantine.platform.Errno;
import java.io.ByteArrayOutputStream;
import java.io.File;
+import java.lang.ref.Reference;
import java.util.EnumSet;
import java.util.concurrent.atomic.AtomicLong;
import org.jruby.ast.RootNode;
@@ -3366,22 +3367,48 @@ public int getFileno() {
}
}
+ public Map<Integer, WeakDescriptorReference> getDescriptors() {
+ return descriptors;
+ }
+
+ private void cleanDescriptors() {
+ Reference reference;
+ while ((reference = descriptorQueue.poll()) != null) {
+ int fileno = ((WeakDescriptorReference)reference).getFileno();
+ descriptors.remove(fileno);
+ }
+ }
+
public void registerDescriptor(ChannelDescriptor descriptor, boolean isRetained) {
+ cleanDescriptors();
+
Integer filenoKey = descriptor.getFileno();
- retainedDescriptors.put(filenoKey, descriptor);
+ descriptors.put(filenoKey, new WeakDescriptorReference(descriptor, descriptorQueue));
+ if (isRetained) {
+ retainedDescriptors.put(filenoKey, descriptor);
+ }
}
public void registerDescriptor(ChannelDescriptor descriptor) {
registerDescriptor(descriptor,false); // default: don't retain
}
public void unregisterDescriptor(int aFileno) {
+ cleanDescriptors();
+
Integer aFilenoKey = aFileno;
+ descriptors.remove(aFilenoKey);
retainedDescriptors.remove(aFilenoKey);
}
public ChannelDescriptor getDescriptorByFileno(int aFileno) {
- return retainedDescriptors.get(aFileno);
+ cleanDescriptors();
+
+ Reference reference = descriptors.get(aFileno);
+ if (reference == null) {
+ return null;
+ }
+ return (ChannelDescriptor)reference.get();
}
public long incrementRandomSeedSequence() {
@@ -3727,7 +3754,10 @@ public Object getHierarchyLock() {
private final ObjectSpace objectSpace = new ObjectSpace();
private final RubySymbol.SymbolTable symbolTable = new RubySymbol.SymbolTable(this);
- private final Map<Integer, ChannelDescriptor> retainedDescriptors = new ConcurrentHashMap<Integer, ChannelDescriptor>();
+ private Map<Integer, WeakDescriptorReference> descriptors = new ConcurrentHashMap<Integer, WeakDescriptorReference>();
+ private ReferenceQueue<ChannelDescriptor> descriptorQueue = new ReferenceQueue<ChannelDescriptor>();
+ // ChannelDescriptors opened by sysopen are cached to avoid collection
+ private Map<Integer, ChannelDescriptor> retainedDescriptors = new ConcurrentHashMap<Integer, ChannelDescriptor>();
private long randomSeed = 0;
private long randomSeedSequence = 0;

0 comments on commit 4760d0a

Please sign in to comment.