diff options
author | Neil Jerram <neil@ossau.uklinux.net> | 2005-03-30 22:11:07 +0000 |
---|---|---|
committer | Neil Jerram <neil@ossau.uklinux.net> | 2005-03-30 22:11:07 +0000 |
commit | 33b320ae2a4f654fdced606ddff09037fd5eaaae (patch) | |
tree | 6acbd8a26a59410f46cbbe94ec1bbabfa1cb33e1 | |
parent | 2b0fb0a50ef538eca51627502616b449e61bee1d (diff) | |
download | guile-33b320ae2a4f654fdced606ddff09037fd5eaaae.tar.gz |
Critical section review.
-rw-r--r-- | libguile/gc.c | 16 | ||||
-rw-r--r-- | libguile/gh.h | 2 |
2 files changed, 16 insertions, 2 deletions
diff --git a/libguile/gc.c b/libguile/gc.c index 4b4c2ea82..81d873edd 100644 --- a/libguile/gc.c +++ b/libguile/gc.c @@ -354,7 +354,8 @@ SCM_DEFINE (scm_gc_stats, "gc-stats", 0, 0, 0, scm_from_ulong (bounds[2*i+1])), heap_segs); } - + /* njrev: can any of these scm_cons's or scm_list_n signal a memory + error? If so we need a frame here. */ answer = scm_list_n (scm_cons (sym_gc_time_taken, scm_from_ulong (local_scm_gc_time_taken)), @@ -443,6 +444,13 @@ SCM_DEFINE (scm_gc, "gc", 0, 0, 0, scm_i_scm_pthread_mutex_lock (&scm_i_sweep_mutex); scm_gc_running_p = 1; scm_i_gc ("call"); + /* njrev: It looks as though other places, e.g. scm_realloc, + can call scm_i_gc without acquiring the sweep mutex. Does this + matter? Also scm_i_gc (or its descendants) touch the + scm_sys_protects, which are protected in some cases + (e.g. scm_permobjs above in scm_gc_stats) by a critical section, + not by the sweep mutex. Shouldn't all the GC-relevant objects be + protected in the same way? */ scm_gc_running_p = 0; scm_i_pthread_mutex_unlock (&scm_i_sweep_mutex); scm_c_hook_run (&scm_after_gc_c_hook, 0); @@ -728,6 +736,8 @@ scm_gc_protect_object (SCM obj) SCM handle; /* This critical section barrier will be replaced by a mutex. */ + /* njrev: Indeed; if my comment above is correct, there is the same + critsec/mutex inconsistency here. */ SCM_CRITICAL_SECTION_START; handle = scm_hashq_create_handle_x (scm_protects, obj, scm_from_int (0)); @@ -751,6 +761,7 @@ scm_gc_unprotect_object (SCM obj) SCM handle; /* This critical section barrier will be replaced by a mutex. */ + /* njrev: and again. */ SCM_CRITICAL_SECTION_START; if (scm_gc_running_p) @@ -788,10 +799,12 @@ scm_gc_register_root (SCM *p) SCM key = scm_from_ulong ((unsigned long) p); /* This critical section barrier will be replaced by a mutex. */ + /* njrev: and again. */ SCM_CRITICAL_SECTION_START; handle = scm_hashv_create_handle_x (scm_gc_registered_roots, key, scm_from_int (0)); + /* njrev: note also that the above can probably signal an error */ SCM_SETCDR (handle, scm_sum (SCM_CDR (handle), scm_from_int (1))); SCM_CRITICAL_SECTION_END; @@ -804,6 +817,7 @@ scm_gc_unregister_root (SCM *p) SCM key = scm_from_ulong ((unsigned long) p); /* This critical section barrier will be replaced by a mutex. */ + /* njrev: and again. */ SCM_CRITICAL_SECTION_START; handle = scm_hashv_get_handle (scm_gc_registered_roots, key); diff --git a/libguile/gh.h b/libguile/gh.h index cc230cfa0..f891f0938 100644 --- a/libguile/gh.h +++ b/libguile/gh.h @@ -231,7 +231,7 @@ SCM_API void gh_newline (void); #endif #endif /* __GH_H */ - +/* njrev: critical sections reviewed so far up to here */ /* Local Variables: c-file-style: "gnu" |