Cleanup and refactor Fortran shared memory usage

also added some missing locking for the ipc(1) value in ft8_decode().

CMakeLists.txt

@@ -224,6 +224,7 @@ set (WSJT_QT_CONF_DESTINATION ${QT_CONF_DESTINATION} CACHE PATH "Path for the qt
 #
 set (wsjt_qt_CXXSRCS
   qt_helpers.cpp
+  lib/shmem.cpp
   widgets/MessageBox.cpp
   MetaDataRegistry.cpp
   Network/NetworkServerLookup.cpp
@@ -298,10 +299,6 @@ set (jt9_FSRCS
   lib/jt9a.f90
   )
 
-set (jt9_CXXSRCS
-  lib/ipcomm.cpp
-  )
-
 set (wsjtx_CXXSRCS
   logbook/logbook.cpp
   Network/psk_reporter.cpp
@@ -360,6 +357,7 @@ endif (WIN32)
 
 set (wsjt_FSRCS
   # put module sources first in the hope that they get rebuilt before use
+  lib/shmem.f90
   lib/crc.f90
   lib/fftw3mod.f90
   lib/hashing.f90
@@ -715,7 +713,6 @@ set (all_CXXSRCS
   ${wsjt_CXXSRCS}
   ${wsjt_qt_CXXSRCS}
   ${wsjt_qtmm_CXXSRCS}
-  ${jt9_CXXSRCS}
   ${wsjtx_CXXSRCS}
   ${qcp_CXXSRCS}
   )
@@ -1221,7 +1218,7 @@ add_executable (fcal lib/fcal.f90 wsjtx.rc)
 
 add_executable (fmeasure lib/fmeasure.f90 wsjtx.rc)
 
-add_executable (jt9 ${jt9_FSRCS} ${jt9_CXXSRCS} wsjtx.rc)
+add_executable (jt9 ${jt9_FSRCS} wsjtx.rc)
 if (${OPENMP_FOUND} OR APPLE)
   if (APPLE)
     # On  Mac  we don't  have  working  OpenMP  support in  the  C/C++
@@ -1254,7 +1251,7 @@ if (${OPENMP_FOUND} OR APPLE)
       LINK_FLAGS -Wl,--stack,16777216
       )
   endif ()
-  target_link_libraries (jt9 wsjt_fort_omp wsjt_cxx Qt5::Core)
+  target_link_libraries (jt9 wsjt_fort_omp wsjt_cxx wsjt_qt)
 else (${OPENMP_FOUND} OR APPLE)
   target_link_libraries (jt9 wsjt_fort wsjt_cxx Qt5::Core)
 endif (${OPENMP_FOUND} OR APPLE)

Detector/Detector.cpp

@@ -11,6 +11,8 @@ extern "C" {
   void   fil4_(qint16*, qint32*, qint16*, qint32*);
 }
 
+extern dec_data_t dec_data;
+
 Detector::Detector (unsigned frameRate, double periodLengthInSeconds,
                     unsigned downSampleFactor, QObject * parent)
   : AudioDevice (parent)

commons.h

@@ -7,9 +7,7 @@
 
 #ifdef __cplusplus
 #include <cstdbool>
-extern "C" {
+#else
-#endif
-#ifndef __cplusplus
 #include <stdbool.h>
 #endif
 
@@ -17,7 +15,7 @@ extern "C" {
    * This structure is shared with Fortran code, it MUST be kept in
    * sync with lib/jt9com.f90
    */
-extern struct dec_data {
+typedef struct dec_data {
   int   ipc[3];
   float ss[184*NSMAX];
   float savg[NSMAX];
@@ -67,7 +65,11 @@ extern struct dec_data {
     char hiscall[12];
     char hisgrid[6];
   } params;
-} dec_data;
+} dec_data_t;
+
+#ifdef __cplusplus
+extern "C" {
+#endif
 
 extern struct {
   float syellow[NSMAX];

getfile.cpp

@@ -20,6 +20,8 @@
 
 #include "commons.h"
 
+extern dec_data dec_data;
+
 void getfile(QString fname, int ntrperiod)
 {
   // struct WAVHDR {

lib/ft8_decode.f90

@@ -35,7 +35,10 @@ contains
   subroutine decode(this,callback,iwave,nQSOProgress,nfqso,nftx,newdat,  &
        nutc,nfa,nfb,nzhsym,ndepth,ncontest,nagain,lft8apon,lapcqonly,    &
        napwid,mycall12,hiscall12,hisgrid6,ipc1,ldiskdat)
+    use iso_c_binding, only: c_bool, c_int
     use timer_module, only: timer
+    use shmem, only: shmem_lock, shmem_unlock
+
     include 'ft8/ft8_params.f90'
 
     class(ft8_decoder), intent(inout) :: this
@@ -49,6 +52,7 @@ contains
     logical, intent(in) :: lft8apon,lapcqonly,nagain
     logical newdat,lsubtract,ldupe,lrefinedt
     logical*1 ldiskdat
+    integer(c_int), volatile, intent(inout) :: ipc1
     logical lsubtracted(MAX_EARLY)
     character*12 mycall12,hiscall12
     character*6 hisgrid6
@@ -62,9 +66,10 @@ contains
     integer itime(8)
     real f1_save(MAX_EARLY)
     real xdt_save(MAX_EARLY)
+    integer(c_int) :: ihsym
+    logical(c_bool) :: ok
 
     save s,dd,dd1,ndec_early,itone_save,f1_save,xdt_save,lsubtracted
-    volatile ipc1
 
     this%callback => callback
     write(datetime,1001) nutc        !### TEMPORARY ###
@@ -90,7 +95,12 @@ contains
                   lrefinedt)
              lsubtracted(i)=.true.
           endif
-          if(.not.ldiskdat .and. ipc1.ge.49) then !Bail out before done
+          ok=shmem_lock()
+          if(.not.ok) call abort
+          ihsym=ipc1                              !read latest from shared memory
+          ok=shmem_unlock()
+          if(.not.ok) call abort
+          if(.not.ldiskdat .and. ihsym.ge.49) then !Bail out before done
              call timer('sub_ft8b',1)
              dd1=dd
              go to 700
@@ -176,8 +186,13 @@ contains
               call this%callback(sync,nsnr,xdt,f1,msg37,iaptype,qual)
            endif
         endif
+        ok=shmem_lock()
+        if(.not.ok) call abort
+        ihsym=ipc1              !read latest from shared memory
+        ok=shmem_unlock()
+        if(.not.ok) call abort
         if(.not.ldiskdat .and. nzhsym.eq.41 .and.                        &
-             ipc1.ge.46) go to 700                 !Bail out before done
+             ihsym.ge.46) go to 700                 !Bail out before done
       enddo
    enddo
    go to 800
@@ -189,7 +204,7 @@ contains
    tseq=mod(itime(7)+0.001*itime(8),15.0)
    if(tseq.lt.9.0) tseq=tseq+15.0
    sec=itime(7)+0.001*itime(8)
-   write(71,3001) 'CC Bailout     ',tsec,nzhsym,ipc1,tseq,     &
+   write(71,3001) 'CC Bailout     ',tsec,nzhsym,ihsym,tseq,     &
         itime(5)-itime(4)/60,itime(6),sec,ndecodes
 3001 format(a15,f11.3,2i6,f8.3,i4.2,':',i2.2,':',f6.3,i6)
    flush(71)

lib/ipcomm.cpp

@@ -1,48 +0,0 @@
-#include <QDebug>
-#include <QString>
-#include <QSharedMemory>
-#include <QSystemSemaphore>
-
-#include "../commons.h"
-
-// Multiple instances: KK1D, 17 Jul 2013
-QSharedMemory mem_jt9;
-
-// Semaphore not changed, as the acquire/release calls do not 
-// appear to be used anywhere.  
-QSystemSemaphore sem_jt9("sem_jt9", 1, QSystemSemaphore::Open);
-
-extern "C" {
-  bool attach_jt9_();
-  bool create_jt9_(int nsize);
-  bool detach_jt9_();
-  bool lock_jt9_();
-  bool unlock_jt9_();
-  struct jt9com * address_jt9_();
-  int size_jt9_();
-// Multiple instances:  wrapper for QSharedMemory::setKey()
-  bool setkey_jt9_(char* mykey, int mykey_len);
-
-  bool acquire_jt9_();
-  bool release_jt9_();
-}
-
-bool attach_jt9_() {return mem_jt9.attach();}
-bool create_jt9_(int nsize) {return mem_jt9.create(nsize);}
-bool detach_jt9_() {return mem_jt9.detach();}
-bool lock_jt9_() {return mem_jt9.lock();}
-bool unlock_jt9_() {return mem_jt9.unlock();}
-struct jt9com * address_jt9_() {return reinterpret_cast<struct jt9com *>(mem_jt9.data());}
-int size_jt9_() {return (int)mem_jt9.size();}
-
-// Multiple instances:
-bool setkey_jt9_(char* mykey, int mykey_len) {
-   char *tempstr = (char *)calloc(mykey_len+1,1);
-   memset(tempstr, 0, mykey_len+1);
-   strncpy(tempstr, mykey, mykey_len);
-   QString s1 = QString(QLatin1String(tempstr));
-   mem_jt9.setKey(s1);
-   return true;}
-
-bool acquire_jt9_() {return sem_jt9.acquire();}
-bool release_jt9_() {return sem_jt9.release();}

lib/jt9a.f90

@@ -1,30 +1,17 @@
 subroutine jt9a()
-  use, intrinsic :: iso_c_binding, only: c_f_pointer
+  use, intrinsic :: iso_c_binding, only: c_f_pointer, c_null_char, c_bool
   use prog_args
   use timer_module, only: timer
   use timer_impl, only: init_timer !, limtrace
+  use shmem
 
   include 'jt9com.f90'
 
-! These routines connect the shared memory region to the decoder.
-  interface
-     function address_jt9()
-       use, intrinsic :: iso_c_binding, only: c_ptr
-       type(c_ptr) :: address_jt9
-     end function address_jt9
-  end interface
-
   integer*2 id2a(180000)
-  integer*1 attach_jt9
-  integer size_jt9
 ! Multiple instances:
-  character*80 mykey
+  type(dec_data), pointer, volatile :: shared_data !also makes target volatile
-  type(dec_data), pointer :: shared_data
   type(params_block) :: local_params
-  volatile shared_data
+  logical(c_bool) :: ok
-
-! Multiple instances:
-  i0 = len(trim(shm_key))
 
   call init_timer (trim(data_dir)//'/timer.out')
 !  open(23,file=trim(data_dir)//'/CALL3.TXT',status='unknown')
@@ -32,36 +19,39 @@ subroutine jt9a()
 !  limtrace=-1                            !Disable all calls to timer()
 
 ! Multiple instances: set the shared memory key before attaching
-  mykey=trim(repeat(shm_key,1))
+  call shmem_setkey(trim(shm_key)//c_null_char)
-  i0 = len(mykey)
+  ok=shmem_attach()
-  i0=setkey_jt9(trim(mykey))
+  if(.not.ok) call abort
-  i1=attach_jt9()
   msdelay=30
-  call c_f_pointer(address_jt9(),shared_data)
+  call c_f_pointer(shmem_address(),shared_data)
 
 ! Wait here until GUI has set ips(2) to 1.0
-10 call lock_jt9()
+10 ok=shmem_lock()
+  if(.not.ok) call abort
   if(shared_data%ipc(2).eq.999.0) then
-     call unlock_jt9()
+     ok=shmem_unlock()
-     i1=detach_jt9()
+     ok=shmem_detach()
      go to 999
   endif
   if(shared_data%ipc(2).ne.1.0) then
-     call unlock_jt9()
+     ok=shmem_unlock()
+     if(.not.ok) call abort
      call sleep_msec(msdelay)
      go to 10
   endif
   shared_data%ipc(2)=0
 
-  nbytes=size_jt9()
+  nbytes=shmem_size()
   if(nbytes.le.0) then
-     call unlock_jt9()
+     ok=shmem_unlock()
-     print*,'jt9a: Shared memory mem_jt9 does not exist.'
+     ok=shmem_detach()
+     print*,'jt9a: Shared memory does not exist.'
      print*,"Must start 'jt9 -s <thekey>' from within WSJT-X."
      go to 999
   endif
   local_params=shared_data%params !save a copy because wsjtx carries on accessing  
-  call unlock_jt9()
+  ok=shmem_unlock()
+  if(.not.ok) call abort
   call flush(6)
   call timer('decoder ',0)
   if(local_params%nmode.eq.8 .and. local_params%ndiskdat) then
@@ -87,14 +77,17 @@ subroutine jt9a()
 
 
 ! Wait here until GUI routine decodeDone() has set ipc(3) to 1.0
-100 call lock_jt9()
+100 ok=shmem_lock()
+  if(.not.ok) call abort
   if(shared_data%ipc(3).ne.1.0) then
-     call unlock_jt9()
+     ok=shmem_unlock()
+     if(.not.ok) call abort
      call sleep_msec(msdelay)
      go to 100
   endif
   shared_data%ipc(3)=0
-  call unlock_jt9()
+  ok=shmem_unlock()
+  if(.not.ok) call abort
   go to 10
   
 999 call timer('decoder ',101)

lib/shmem.cpp

@@ -0,0 +1,20 @@
+#include <QSharedMemory>
+#include <QLatin1String>
+
+// Multiple instances: KK1D, 17 Jul 2013
+QSharedMemory shmem;
+
+struct jt9com;
+
+// C wrappers for a QSharedMemory class instance
+extern "C"
+{
+  bool shmem_create (int nsize) {return shmem.create(nsize);}
+  void shmem_setkey (char * const mykey) {shmem.setKey(QLatin1String{mykey});}
+  bool shmem_attach () {return shmem.attach();}
+  int shmem_size () {return static_cast<int> (shmem.size());}
+  struct jt9com * shmem_address () {return reinterpret_cast<struct jt9com *>(shmem.data());}
+  bool shmem_lock () {return shmem.lock();}
+  bool shmem_unlock () {return shmem.unlock();}
+  bool shmem_detach () {return shmem.detach();}
+}

widgets/mainwindow.cpp

@@ -178,7 +178,7 @@ extern "C" {
 int volatile itone[NUM_ISCAT_SYMBOLS];   //Audio tones for all Tx symbols
 int volatile itone0[NUM_ISCAT_SYMBOLS];  //Dummy array, data not actually used
 int volatile icw[NUM_CW_SYMBOLS];        //Dits for CW ID
-struct dec_data dec_data;                // for sharing with Fortran
+dec_data_t dec_data;                // for sharing with Fortran
 
 int outBufSize;
 int rc;
@@ -3072,13 +3072,11 @@ void::MainWindow::fast_decode_done()
 
 void MainWindow::to_jt9(qint32 n, qint32 istart, qint32 idone)
 {
-  int ipc[3];
+  dec_data_t * dd = reinterpret_cast<dec_data_t *> (mem_jt9->data());
   mem_jt9->lock ();
-  memcpy(ipc,(char*)mem_jt9->data(),12);
+  dd->ipc[0]=n;
-  ipc[0]=n;
+  if(istart>=0) dd->ipc[1]=istart;
-  if(istart>=0) ipc[1]=istart;
+  if(idone>=0)  dd->ipc[2]=idone;
-  if(idone>=0)  ipc[2]=idone;
-  memcpy((char*)mem_jt9->data(),ipc,12);
   mem_jt9->unlock ();
 }
 

widgets/plotter.cpp

@@ -19,6 +19,8 @@ extern "C" {
   void plotsave_(float swide[], int* m_w , int* m_h1, int* irow);
 }
 
+extern dec_data dec_data;
+
 CPlotter::CPlotter(QWidget *parent) :                  //CPlotter Constructor
   QFrame {parent},
   m_set_freq_action {new QAction {tr ("&Set Rx && Tx Offset"), this}},