Passa codice

Leggendo il blog di Coding Horror, ho avuto occasione di riflettere sulla seguente frase:

Always code as if the person who ends up maintaining your code is a violent psychopath who knows where you live.

È un paio di giorni che mi sento un po’ shining:

the_shining

L’aggiunta di una piccola feature nel prodotto a cui sto lavorando, così piccola che si può descrivere con pochissime pagine fin nei minimi dettagli, mi ha costretto a riscrivere quasi ex-novo del codice non mio. Ho pensato che se mi avessero dato una mazza da baseball ne avrei combinata qualcuna delle mie: poi mi sono decisamente calmato quando ho preso in considerazione la scarsa esperienza iniziale di chi ha scritto quel codice. Non che il codice sia terribile, ma diciamo che il refactoring è diventato un pretesto per fare di necessità virtù e tirare fuori un paio di query micidiali fuori da un loop ammazza prestazioni.

E a proposito di codice, lascio un quesito per i volenterosi ricavato dall’esperienza con blogoo: la classe SyncLRUMap in questo pezzo di codice “autosufficiente” di C# ha qualche problema di multi-threading che si verifica abbastanza sporadicamente. Quando si verifica un eccezione di tipo Null Reference viene generata ed il call stack è il seguente:

System.NullReferenceException:
Object reference not set to an instance of an object.
at Commons.Collections.LRUMap.IndexOf(Object key)
at Commons.Collections.LRUMap.MoveToMRU(Object key)
at Commons.Collections.LRUMap.get_Item(Object key)
at Commons.Collections.LRUMap.SyncLRUMap.get_Item(Object key)

Una volta che si verifica il guaio la collezione risulta permanentemente corrotta, ovvero l’oggetto membro ObjectList finisce per contenere un null. Un indizio per la soluzione (che io spero di aver individuato) è indicato in questo thread. Un ulteriore indizio lo si può individuare riflettendo sull’implementazione di ArrayList. Spero di aver stuzzicato abbastanza curiosità.

-quack

Technorati Tags:

Potrebbero interessarti anche:
Commenti (6):
1. Kaone
giovedì 26 giugno 2008 alle 1:04 PM - unknown unknown unknown
   

Bellissima! :-DDDDD

Me la metto come tagline in Messenger!

   
2. Blackstorm
giovedì 26 giugno 2008 alle 2:28 PM - unknown unknown unknown
   

Sempre grande il nostro Jeff...

   
3. EnricoG
giovedì 26 giugno 2008 alle 7:22 PM - unknown unknown unknown
   

Anche se si passa per SyncLRUMap.get_Item, in realta' MoveToMRU viene invocato senza alcuna sincronizzazione.

Cosa poi combini ArrayList internamente puo' essere interessante, ma fino ad un certo punto, perche' mi pare chiaro che il problema e' che get_Item non e' multi-thread safe.

   
4. Paperino
giovedì 26 giugno 2008 alle 8:13 PM - unknown unknown unknown
   

@EnricoG:

Quello che combina ArrayList è interessante perché se si guarda i metodi invocati in MoveToMRU, e cioé RemoveAt e InsertAt, sembrerebbe impossibile che nella lista ci finisca un null.

Perché la cosa buffa è che se non ci fosse questo problema (e cioé che la lista diventa corrotta) tutto continuerebbe a funzionare o per lo meno a sembrare che funzioni: infatti se un elemento non avanza correttamente nella lista dei MRU la funzionalità non viene pregiudicata.

Per questo i tipi nel forum stavano impazzendo nel cercare di riprodurre il problema in maniera controllata. Impossibile senza una serie di Sleep Injections (TM)

   
5. EnricoG
venerdì 27 giugno 2008 alle 10:23 AM - unknown unknown unknown
   

Infatti e' positivo che ArrayList finisca per causare un'eccezione visto che cosi' viene esposto in modo inequivocabile che il codice chiamante non e' thread-safe.

A questo punto, anche senza pubblicare il codice, potresti dirci se secondo te quello che combina ArrayList e' voluto o dovuto solo ad una fortunosa coincidenza

   
6. Paperino
venerdì 27 giugno 2008 alle 7:09 PM - unknown unknown unknown
   

@EnricoG:

estremamente fortuito. Anzi affinché la corruption si verifichi i due thread devono allinearsi entro qualche decina di righe di codice.

   
Lascia un commento:
Commento: (clicca su questo link per gli smiley supportati; regole di ingaggio per i commenti)
(opzionale, per il Gravatar)