GDP

The problem seems to be that if I run the model twice, then it sometimes crashes because close() is called twice.

To replicate in Ptolemy, run

$PTII/bin/vergil $PTII/ptolemy/actor/lib/jjs/modules/gdp/test/auto/GDPLogCreateAppendReadJS.xml

Here is the end of a successful run

>>> gdp_gcl_close(m5FiAV65ufV8Oe3SinfrOZPcbzXtlNOG-lmD-KSsE1U)
gdp_gcl_ops.c459: _gdp_gcl_close(): gcl is not null
<<< gdp_gcl_close(m5FiAV65ufV8Oe3SinfrOZPcbzXtlNOG-lmD-KSsE1U): OK
13968 ms. Memory: 544256K Free: 210047K (39%)

The message about gcl is not null is because I modified gdp_gcl_ops.c so that it prints that message:

EP_STAT
_gdp_gcl_close(gdp_gcl_t *gcl,
                        gdp_chan_t *chan,
                        uint32_t reqflags)
{
        EP_STAT estat = EP_STAT_OK;
        gdp_req_t *req;
    int nrefs;

        errno = 0;                              // avoid spurious messages                                         
    if (gcl == NULL) {
          fprintf(stderr, "%s%d: _gdp_gcl_close(): gcl is null\n", __FILE__, __LINE__);
        } else {
          fprintf(stderr, "%s%d: _gdp_gcl_close(): gcl is not null\n", __FILE__, __LINE__);
        }
        GDP_ASSERT_GOOD_GCL(gcl);

Here is the continuation of the run, just after the 13968 ms. line above:

GDP_GCL.close(): gclh: native@0x7ffc5cc6cc00
GDP_GCL.close(): after removing gclh, gclh: native@0x7ffc5cc6cc00

>>> gdp_gcl_close(m5FiAV65ufV8Oe3SinfrOZPcbzXtlNOG-lmD-KSsE1U)
gdp_gcl_ops.c459: _gdp_gcl_close(): gcl is not null

>>> _gdp_invoke(req=0x7ffc5cb298b0 rid=0): CMD_CLOSE (69), gcl@0x7ffc5cc6cc00
        datum @ 0x7ffc5c4f9c30: recno -1, len 0, no timestamp
_gdp_pdu_out, fd = 120, basemd = 0x0: CMD_CLOSE
_gdp_pdu_in(ACK_SUCCESS) => OK

*** Processing ack/nak 128=ACK_SUCCESS from socket 120
gdp_pdu_proc_resp(0x7ffc5c88d060 ACK_SUCCESS) gcl 0x7ffc5cc6cc00
_gdp_req_dispatch >>> ACK_SUCCESS (128) [gcl->refcnt 2]
ack_success: received ACK_SUCCESS for CMD_CLOSE
_gdp_req_dispatch <<< ACK_SUCCESS [gcl->refcnt 2]
    OK [67896520 = 0x40c04c8]
<<< _gdp_invoke(0x7ffc5cb298b0 rid=0) CMD_CLOSE: OK [67896520 = 0x40c04c8]
<<< gdp_gcl_close(m5FiAV65ufV8Oe3SinfrOZPcbzXtlNOG-lmD-KSsE1U): OK [67896520 = 0x40c04c8]
java.lang.Exception: GDP_GCL.close() was called
    at org.terraswarm.gdp.GDP_GCL.close(GDP_GCL.java:222)
        at org.terraswarm.gdp.GDP_GCL.finalize(GDP_GCL.java:298)
        at java.lang.System$2.invokeFinalize(System.java:1270)
        at java.lang.ref.Finalizer.runFinalizer(Finalizer.java:98)
        at java.lang.ref.Finalizer.access$100(Finalizer.java:34)
        at java.lang.ref.Finalizer$FinalizerThread.run(Finalizer.java:210)
GDP_GCL.close(): gclh: native@0x7ffc5cc6cc00
GDP_GCL.close(): after removing gclh, gclh: native@0x7ffc5cc6cc00

>>> gdp_gcl_close(m5FiAV65ufV8Oe3SinfrOZPcbzXtlNOG-lmD-KSsE1U)
gdp_gcl_ops.c459: _gdp_gcl_close(): gcl is not null
Assertion failed at gdp_gcl_ops.c:461: require:
        (gcl) != NULL && EP_UT_BITSET(GCLF_INUSE, (gcl)->flags)
Abort trap: 6
bash-3.2$

The GDP_GCL messages are from gdp/lang/java/org/terraswarm/gdp/GDP_GCL.java

    /** Close the GCL.                                                                                             
     */
    public void close() {
        new Exception("GDP_GCL.close() was called").printStackTrace();
        System.out.println("GDP_GCL.close(): gclh: " + gclh);
        // Remove ourselves from the global list.                                                                  
        _allGclhs.remove(gclh);

        System.out.println("GDP_GCL.close(): after removing gclh, gclh: " + gclh);
        // Free the associated gdp_gcl_t.                                                                          
        Gdp07Library.INSTANCE.gdp_gcl_close(gclh);
    }

What's happening here is that the GDP_GCL Java object is being finalized so close() is being called again.

Calling close() with a gcl that has already been closed should not ever fail.

Passing null or any other garbage as a gcl should also not ever fail.

Take a look at how fuzz testing works. We should expect the user to pass anything as an argument and the gdp library code should detect that and return an error.

Have a library exit the process is overkill. An absurd extension would be to have the process reformat the hard drive because of an error like this.

The best approach would be to return an error value.

A big problem with crashing the process is that I can't put the close() in a Java try block. I think that a good library always allows the user to try to close something multiple times and at worst returns a message that says that it was already closed.

If the GDP is going to be used successfully as a library it needs to be more forgiving about how it is used. Crashing the process is not user-friendly.

As of commit:8701c522 gdp_gcl_close should return GDP_STAT_GCL_NOT_OPEN when passed a bad GCL.