Skip to content

Commit 0574a0d

Browse files
Alexander--Alexander--
committed
More reliable closing of factory. Released 0.2.4
1 parent 1541be2 commit 0574a0d

7 files changed

Lines changed: 122 additions & 46 deletions

File tree

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ Add following dependency to your `dependencies` block:
6363
```groovy
6464
dependencies {
6565
...
66-
compile 'net.sf.fdshare:library:0.2.2@aar'
66+
compile 'net.sf.fdshare:library:0.2.4@aar'
6767
}
6868
```
6969

‎example/build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ dependencies {
4848
compile project(':library')
4949
} else {
5050
debugCompile project(path: ':library', configuration: 'debug')
51-
releaseCompile 'net.sf.fdshare:library:0.2.1@aar'
51+
releaseCompile 'net.sf.fdshare:library:0.2.4@aar'
5252
}
5353

5454
compile 'com.j256.simplemagic:simplemagic:1.6'

‎example/src/main/java/net/sf/fdshare/RootFileProvider.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import android.os.Build;
2323
import android.os.CancellationSignal;
2424
import android.os.ParcelFileDescriptor;
25+
import android.support.v4.app.FragmentPagerAdapter;
2526
import android.text.TextUtils;
2627
import android.util.Log;
2728

‎library/build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ idea {
1717
}
1818

1919
group = 'net.sf.fdshare'
20-
version = '0.2.2'
20+
version = '0.2.4'
2121

2222
android {
2323
compileSdkVersion 21 // mainly for try-with-resources

‎library/proguard-rules.pro‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,5 @@
2323
public *;
2424
}
2525

26-
-assumenosideeffects class * {
27-
!public !private static FileDescriptorFactory createTest(android.content.Context);
28-
}
29-
3026
# Retrolambda, per https://github.com/evant/gradle-retrolambda
3127
-dontwarn java.lang.invoke.*

‎library/src/androidTest/java/net/sf/fdshare/LibraryTest.java‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package net.sf.fdshare;
22

33
import android.annotation.TargetApi;
4+
import android.content.Context;
45
import android.os.ParcelFileDescriptor;
56
import android.support.test.InstrumentationRegistry;
67
import android.support.test.runner.AndroidJUnit4;
@@ -11,6 +12,13 @@
1112

1213
import java.io.File;
1314
import java.io.IOException;
15+
import java.io.PrintWriter;
16+
import java.io.RandomAccessFile;
17+
import java.nio.MappedByteBuffer;
18+
import java.nio.channels.ByteChannel;
19+
import java.nio.channels.Channel;
20+
import java.nio.channels.FileChannel;
21+
import java.nio.charset.CharsetEncoder;
1422

1523
@RunWith(AndroidJUnit4.class)
1624
@TargetApi(22)
@@ -48,4 +56,39 @@ public void testAbleToOpenWellKnownFile() throws IOException, FactoryBrokenExcep
4856
Assert.assertEquals(fd.getStatSize(), exec.length());
4957
}
5058
}
59+
60+
@Test(expected = IOException.class)
61+
public void testUnableToOpenRandomAccessFile() throws IOException, FactoryBrokenException {
62+
try (FileDescriptorFactory fdf = FileDescriptorFactory.create(InstrumentationRegistry.getContext()))
63+
{
64+
// opening with read-write permission with fail :(
65+
final RandomAccessFile fd = fdf.openRandomAccessFile(exec);
66+
}
67+
}
68+
69+
@Test
70+
public void testAbleToOpenRandomAccessFile() throws IOException, FactoryBrokenException {
71+
final Context context = InstrumentationRegistry.getContext();
72+
73+
final File tmpFile = File.createTempFile("test", null, context.getFilesDir());
74+
final PrintWriter out = new PrintWriter(tmpFile);
75+
out.write("TEST");
76+
out.close();
77+
78+
try (FileDescriptorFactory fdf = FileDescriptorFactory.create(context))
79+
{
80+
final RandomAccessFile fd = fdf.openRandomAccessFile(tmpFile);
81+
82+
final MappedByteBuffer mbb = fd.getChannel().map(FileChannel.MapMode.READ_WRITE, 0, tmpFile.length());
83+
mbb.put(2, (byte) '!');
84+
85+
final byte[] dst = new byte["TEST".length()];
86+
mbb.get(dst);
87+
88+
Assert.assertEquals("TE!T", new String(dst, 0, dst.length));
89+
}
90+
91+
//noinspection ResultOfMethodCallIgnored
92+
tmpFile.delete();
93+
}
5194
}

‎library/src/main/java/net/sf/fdshare/FileDescriptorFactory.java‎

Lines changed: 75 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -16,44 +16,42 @@
1616
package net.sf.fdshare;
1717

1818
import android.annotation.TargetApi;
19-
import android.content.ContentResolver;
2019
import android.content.Context;
2120
import android.net.LocalServerSocket;
2221
import android.net.LocalSocket;
2322
import android.os.*;
24-
import android.support.annotation.BinderThread;
2523
import android.support.annotation.IntDef;
2624
import android.support.annotation.NonNull;
2725
import android.support.annotation.VisibleForTesting;
28-
import android.support.annotation.WorkerThread;
2926
import android.util.Log;
3027

3128
import java.io.Closeable;
3229
import java.io.File;
3330
import java.io.FileDescriptor;
3431
import java.io.FileInputStream;
35-
import java.io.FileNotFoundException;
3632
import java.io.FileOutputStream;
3733
import java.io.IOException;
34+
import java.io.InputStream;
35+
import java.io.OutputStream;
3836
import java.io.OutputStreamWriter;
3937
import java.io.RandomAccessFile;
4038
import java.io.Writer;
4139
import java.lang.Process;
4240
import java.lang.annotation.Documented;
4341
import java.lang.annotation.Retention;
4442
import java.lang.annotation.RetentionPolicy;
45-
import java.lang.ref.PhantomReference;
4643
import java.lang.reflect.Field;
4744
import java.nio.ByteBuffer;
4845
import java.nio.ByteOrder;
4946
import java.nio.channels.Channels;
50-
import java.nio.channels.FileChannel;
51-
import java.nio.channels.Pipe;
5247
import java.nio.channels.ReadableByteChannel;
5348
import java.util.UUID;
49+
import java.util.concurrent.ArrayBlockingQueue;
50+
import java.util.concurrent.BlockingQueue;
5451
import java.util.concurrent.SynchronousQueue;
5552
import java.util.concurrent.TimeUnit;
5653
import java.util.concurrent.atomic.AtomicBoolean;
54+
import java.util.concurrent.locks.Condition;
5755
import java.util.regex.Matcher;
5856
import java.util.regex.Pattern;
5957

@@ -196,38 +194,43 @@ public static FileDescriptorFactory create(Context context) throws IOException {
196194

197195
@VisibleForTesting
198196
static FileDescriptorFactory create(String address, String... cmd) throws IOException {
199-
final FileDescriptorFactory result = new FileDescriptorFactory(address);
200-
197+
// must be created before the process
198+
final LocalServerSocket socket = new LocalServerSocket(address);
201199
try {
202200
final Process shell = new ProcessBuilder(cmd)
203201
.redirectErrorStream(true)
204202
.start();
205203

206-
result.startServer(shell);
204+
final FileDescriptorFactory result = new FileDescriptorFactory(shell, socket);
205+
206+
result.startServer();
207207

208208
return result;
209209
} catch (Throwable t) {
210-
result.close();
210+
shut(socket);
211211

212212
throw t;
213213
}
214214
}
215215

216216
private final AtomicBoolean closedStatus = new AtomicBoolean(false);
217-
private final SynchronousQueue<FdReq> intake = new SynchronousQueue<>();
217+
private final ArrayBlockingQueue<FdReq> intake = new ArrayBlockingQueue<>(1);
218218
private final SynchronousQueue<FdResp> responses = new SynchronousQueue<>();
219-
private PhantomReference f;
220219

221-
private final LocalServerSocket socket;
220+
final LocalServerSocket serverSocket;
221+
final Process clientProcess;
222222

223223
private volatile Server serverThread;
224224

225-
private FileDescriptorFactory(String address) throws IOException {
226-
this.socket = new LocalServerSocket(address);
225+
private FileDescriptorFactory(final Process clientProcess, final LocalServerSocket serverSocket) {
226+
this.clientProcess = clientProcess;
227+
this.serverSocket = serverSocket;
228+
229+
intake.offer(FdReq.PLACEHOLDER);
227230
}
228231

229-
private void startServer(Process client) throws IOException {
230-
serverThread = new Server(client, socket);
232+
private void startServer() throws IOException {
233+
serverThread = new Server();
231234
serverThread.start();
232235
}
233236

@@ -281,6 +284,7 @@ private void startServer(Process client) throws IOException {
281284

282285
final FdReq request = new FdReq(path, mode);
283286

287+
284288
FdResp response;
285289
try {
286290
if (intake.offer(request, HELPER_TIMEOUT, TimeUnit.MILLISECONDS)
@@ -310,15 +314,16 @@ private void startServer(Process client) throws IOException {
310314
@Override
311315
public void close() {
312316
if (!closedStatus.compareAndSet(false, true)) {
313-
intake.offer(FdReq.STOP);
317+
shut(clientProcess);
318+
shut(serverSocket);
314319

315-
try {
316-
socket.close();
317-
} catch (IOException e) {
318-
logException("Failed to close server socket", e);
319-
} finally {
320-
if (serverThread != null)
321-
serverThread.interrupt();
320+
if (serverThread != null) {
321+
serverThread.interrupt();
322+
323+
do {
324+
intake.clear();
325+
}
326+
while (!intake.offer(FdReq.STOP));
322327
}
323328
}
324329
}
@@ -328,27 +333,21 @@ public boolean isClosed() {
328333
}
329334

330335
private final class Server extends Thread {
331-
private final Process client;
332-
private final LocalServerSocket serverSocket;
333-
334336
private final ByteBuffer statusMsg = ByteBuffer.allocate(512).order(ByteOrder.nativeOrder());
335337

336338
int lastClientReadCount;
337339

338-
Server(Process client, LocalServerSocket serverSocket) throws IOException {
340+
Server() throws IOException {
339341
super("fd receiver");
340-
341-
this.client = client;
342-
this.serverSocket = serverSocket;
343342
}
344343

345344
@Override
346345
public void run() {
347-
try (ReadableByteChannel clientOutput = Channels.newChannel(client.getInputStream());
346+
try (ReadableByteChannel clientOutput = Channels.newChannel(clientProcess.getInputStream());
348347
Closeable c = new CloseableSocket(serverSocket))
349348
{
350349
try {
351-
initializeAndHandleRequests(readHelperPid(clientOutput), serverSocket);
350+
initializeAndHandleRequests(readHelperPid(clientOutput));
352351
} finally {
353352
try {
354353
do {
@@ -367,7 +366,9 @@ public void run() {
367366
FdCompat.set(closedStatus);
368367

369368
try {
370-
client.waitFor();
369+
setName("BUG: Waiting for su process, which won't quit");
370+
371+
clientProcess.waitFor();
371372
} catch (InterruptedException e) {
372373
Thread.currentThread().interrupt();
373374
}
@@ -389,9 +390,9 @@ private int readHelperPid(ReadableByteChannel clientOutput) throws IOException {
389390
return Integer.valueOf(m.group(1));
390391
}
391392

392-
private void initializeAndHandleRequests(int helperPid, LocalServerSocket socket) throws Exception {
393+
private void initializeAndHandleRequests(int helperPid) throws Exception {
393394
while (!isInterrupted()) {
394-
try (LocalSocket localSocket = socket.accept())
395+
try (LocalSocket localSocket = serverSocket.accept())
395396
{
396397
final int socketPid = localSocket.getPeerCredentials().getPid();
397398
if (socketPid != helperPid)
@@ -430,6 +431,9 @@ private void initializeAndHandleRequests(int helperPid, LocalServerSocket socket
430431
}
431432
}
432433

434+
if (intake.take() == FdReq.STOP)
435+
return;
436+
433437
processRequestsUntilStopped(localSocket, status, clientTty);
434438

435439
break;
@@ -442,7 +446,7 @@ private void initializeAndHandleRequests(int helperPid, LocalServerSocket socket
442446
private void processRequestsUntilStopped(LocalSocket fdrecv, ReadableByteChannel status, Writer control) throws IOException, InterruptedException {
443447
FdReq fileOps;
444448

445-
while ((fileOps = intake.take()) != FdReq.STOP) {
449+
while ((fileOps = intake.poll()) != FdReq.STOP) {
446450
FdResp response = null;
447451

448452
try {
@@ -511,9 +515,41 @@ private static void logException(String explanation, Exception err) {
511515
}
512516
}
513517

518+
private static void shut(Closeable closeable) {
519+
try {
520+
if (closeable != null)
521+
closeable.close();
522+
} catch (IOException e) {
523+
// just as planned
524+
}
525+
}
526+
527+
private static void shut(LocalServerSocket sock) {
528+
try {
529+
if (sock != null)
530+
sock.close();
531+
} catch (IOException e) {
532+
logException("Failed to close server socket", e);
533+
}
534+
}
535+
536+
private static void shut(Process proc) {
537+
try {
538+
if (proc != null) {
539+
shut(proc.getInputStream());
540+
shut(proc.getOutputStream());
541+
proc.destroy();
542+
}
543+
} catch (Exception e) {
544+
// just as planned
545+
}
546+
}
547+
514548
private static final class FdReq {
515549
static FdReq STOP = new FdReq(null, 0);
516550

551+
static FdReq PLACEHOLDER = new FdReq(null, 0);
552+
517553
final String fileName;
518554
final int mode;
519555

0 commit comments

Comments
 (0)