Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit df4c156e authored by Xiao Ma's avatar Xiao Ma
Browse files

Optimize FdEventsReader to make it safer to use. 

Update the start() and createAndRegisterFd() to boolean type, to make
sure we won't return null socket when failing to create socket.

Throwing IllegalStateException if any caller is calling the FdEventsReader#
start/stop from the different thread, to make it immediately obvious.

Bug: None
Test: atest NetworkStackTests
Change-Id: I398b24da34b8fc9da7e8a9d07508c4d731c4a437
parent 3c9f687c

src/android/net/util/FdEventsReader.java

+13 −19
Original line number Diff line number Diff line
@@ -63,7 +63,6 @@ import java.io.IOException; 
 * the Handler constructor argument is associated.

 *

 * @param <BufferType> the type of the buffer used to read data.

 * @hide

 */

public abstract class FdEventsReader<BufferType> {

    private static final int FD_EVENTS = EVENT_INPUT | EVENT_ERROR;
@@ -93,27 +92,21 @@ public abstract class FdEventsReader<BufferType> { 
    }



    /** Start this FdEventsReader. */

    public void start() {

        if (onCorrectThread()) {

            createAndRegisterFd();

        } else {

            mHandler.post(() -> {

                logError("start() called from off-thread", null);

                createAndRegisterFd();

            });

    public boolean start() {

        if (!onCorrectThread()) {

            throw new IllegalStateException("start() called from off-thread");

        }



        return createAndRegisterFd();

    }



    /** Stop this FdEventsReader and destroy the file descriptor. */

    public void stop() {

        if (onCorrectThread()) {

            unregisterAndDestroyFd();

        } else {

            mHandler.post(() -> {

                logError("stop() called from off-thread", null);

                unregisterAndDestroyFd();

            });

        if (!onCorrectThread()) {

            throw new IllegalStateException("stop() called from off-thread");

        }



        unregisterAndDestroyFd();

    }



    @NonNull
@@ -178,8 +171,8 @@ public abstract class FdEventsReader<BufferType> { 
     */

    protected void onStop() {}



    private void createAndRegisterFd() {

        if (mFd != null) return;

    private boolean createAndRegisterFd() {

        if (mFd != null) return true;



        try {

            mFd = createFd();
@@ -189,7 +182,7 @@ public abstract class FdEventsReader<BufferType> { 
            mFd = null;

        }



        if (mFd == null) return;

        if (mFd == null) return false;



        mQueue.addOnFileDescriptorEventListener(

                mFd,
@@ -205,6 +198,7 @@ public abstract class FdEventsReader<BufferType> { 
                    return FD_EVENTS;

                });

        onStart();

        return true;

    }



    private boolean isRunning() {

src/android/net/util/PacketReader.java

+0 −2
Original line number Diff line number Diff line
@@ -28,8 +28,6 @@ import java.io.FileDescriptor; 
 *

 * TODO: rename this class to something more correctly descriptive (something

 * like [or less horrible than] FdReadEventsHandler?).

 *

 * @hide

 */

public abstract class PacketReader extends FdEventsReader<byte[]> {

tests/unit/src/android/net/ip/IpReachabilityMonitorTest.java

+3 −1
Original line number Diff line number Diff line
@@ -62,6 +62,8 @@ public class IpReachabilityMonitorTest { 


    @Test

    public void testNothing() {

        IpReachabilityMonitor monitor = makeMonitor();

        // make sure the unit test runs in the same thread with main looper.

        // Otherwise, throwing IllegalStateException would cause test fails.

        mHandler.post(() -> makeMonitor());

    }

}

tests/unit/src/android/net/util/PacketReaderTest.java

+31 −1
Original line number Diff line number Diff line
@@ -24,6 +24,8 @@ import static android.system.OsConstants.SOCK_NONBLOCK; 
import static android.system.OsConstants.SOL_SOCKET;

import static android.system.OsConstants.SO_SNDTIMEO;



import static com.android.testutils.MiscAssertsKt.assertThrows;



import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertFalse;

import static org.junit.Assert.assertTrue;
@@ -182,7 +184,7 @@ public class PacketReaderTest { 
        assertTrue(Arrays.equals(two, mLastRecvBuf));

        assertFalse(mStopped);



        mReceiver.stop();

        h.post(() -> mReceiver.stop());

        waitForActivity();

        assertEquals(2, mReceiver.numPacketsReceived());

        assertTrue(Arrays.equals(two, mLastRecvBuf));
@@ -208,4 +210,32 @@ public class PacketReaderTest { 
            assertEquals(DEFAULT_RECV_BUF_SIZE, b.recvBufSize());

        }

    }



    @Test

    public void testStartingFromWrongThread() throws Exception {

        final Handler h = mHandlerThread.getThreadHandler();

        final PacketReader b = new NullPacketReader(h, DEFAULT_RECV_BUF_SIZE);

        assertThrows(IllegalStateException.class, () -> b.start());

    }



    @Test

    public void testStoppingFromWrongThread() throws Exception {

        final Handler h = mHandlerThread.getThreadHandler();

        final PacketReader b = new NullPacketReader(h, DEFAULT_RECV_BUF_SIZE);

        assertThrows(IllegalStateException.class, () -> b.stop());

    }



    @Test

    public void testSuccessToCreateSocket() throws Exception {

        final Handler h = mHandlerThread.getThreadHandler();

        final PacketReader b = new UdpLoopbackReader(h);

        h.post(() -> assertTrue(b.start()));

    }



    @Test

    public void testFailToCreateSocket() throws Exception {

        final Handler h = mHandlerThread.getThreadHandler();

        final PacketReader b = new NullPacketReader(h, DEFAULT_RECV_BUF_SIZE);

        h.post(() -> assertFalse(b.start()));

    }

}